[U-Boot] [PATCH PATCH v1] watchdog: Fix Watchdog Reset while in U-Boot Prompt
Hardware: CM-FX6 Module from Compulab
This patch fixes unwanted watchdog resets while the user enters a command at the U-Boot prompt.
As found on the CM-FX6 board from Compulab, when having enabled the watchdog, a missing WATCHDOG_RESET call in common/console.c causes this and alike boards to reset when the watchdog's timeout has elapsed while waiting at the U-Boot prompt.
Despite the user could press several keys within the watchdog timeout limit, the while loop in cli_readline.c, line 261, does only call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st loop iteration. This leads to a watchdog timeout no matter if the user presses keys or not.
Although, this affects other boards as well as it touches common/console.c, the macro WATCHDOG_RESET expands to {} if watchdog support isn't configured. Hence, there's no harm caused and no need to surround it by #ifdef in this case.
* Symptom: U-Boot resets after watchdog times out when in commandline prompt and watchdog is enabled.
* Reasoning: When U-Boot shows the commandline prompt, the following function call stack is executed while waiting for a keypress:
common/main.c: main_loop => common/cli.c: cli_loop() => common/cli_hush.c: parse_file_outer => parse_stream_outer => parse_stream => b_getch(i) => i->get(i) => file_get => get_user_input => cmdedit_read_input => uboot_cli_readline => common/cli_readline.c: cli_readline => cli_readline_into_buffer => cread_line => getcmd_getch (== getc) => common/console.c: fgetc => console_tstc
common/console.c: (with CONFIG_CONSOLE_MUX is set)
- in console_tstc line 181: If dev->tstc(dev) returns 0, the global tstcdev variable doesn't get set. This is the case if no character is in the serial buffer.
- in fgetc(int file), line 297: Program flow keeps looping because tstcdev does not get set. Therefore WATCHDOG_RESET is not called, as mx_serial_tstc from drivers/serial/serial_mxc.c does not call it.
* Solution: Add WATCHDOG_RESET into the loop of console_tstc.
Note: Macro expands to {} if not configured, so no #ifdef is needed.
* Comment:
Signed-off-by: Christian Storm christian.storm@tngtech.com Signed-off-by: Jan Kiszka jan.kiszka@siemens.com Signed-off-by: Andreas J. Reichel Andreas.Reichel@tngtech.com ---
common/console.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/common/console.c b/common/console.c index 12293f3..054b6cd 100644 --- a/common/console.c +++ b/common/console.c @@ -16,6 +16,7 @@ #include <stdio_dev.h> #include <exports.h> #include <environment.h> +#include <watchdog.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -294,6 +295,7 @@ int fgetc(int file) * Effectively poll for input wherever it may be available. */ for (;;) { + WATCHDOG_RESET(); /* * Upper layer may have already called tstc() so * check for that first.
Hi,
On 13 July 2016 at 04:56, Andreas J. Reichel Andreas.Reichel@tngtech.com wrote:
Hardware: CM-FX6 Module from Compulab
This patch fixes unwanted watchdog resets while the user enters a command at the U-Boot prompt.
As found on the CM-FX6 board from Compulab, when having enabled the watchdog, a missing WATCHDOG_RESET call in common/console.c causes this and alike boards to reset when the watchdog's timeout has elapsed while waiting at the U-Boot prompt.
Despite the user could press several keys within the watchdog timeout limit, the while loop in cli_readline.c, line 261, does only call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st loop iteration. This leads to a watchdog timeout no matter if the user presses keys or not.
Although, this affects other boards as well as it touches common/console.c, the macro WATCHDOG_RESET expands to {} if watchdog support isn't configured. Hence, there's no harm caused and no need to surround it by #ifdef in this case.
Symptom: U-Boot resets after watchdog times out when in commandline prompt and watchdog is enabled.
Reasoning: When U-Boot shows the commandline prompt, the following function call stack is executed while waiting for a keypress:
common/main.c: main_loop => common/cli.c: cli_loop() => common/cli_hush.c: parse_file_outer => parse_stream_outer => parse_stream => b_getch(i) => i->get(i) => file_get => get_user_input => cmdedit_read_input => uboot_cli_readline => common/cli_readline.c: cli_readline => cli_readline_into_buffer => cread_line => getcmd_getch (== getc) => common/console.c: fgetc => console_tstc
common/console.c: (with CONFIG_CONSOLE_MUX is set)
- in console_tstc line 181:
If dev->tstc(dev) returns 0, the global tstcdev variable doesn't get set. This is the case if no character is in the serial buffer.
- in fgetc(int file), line 297:
Program flow keeps looping because tstcdev does not get set. Therefore WATCHDOG_RESET is not called, as mx_serial_tstc from drivers/serial/serial_mxc.c does not call it.
Solution: Add WATCHDOG_RESET into the loop of console_tstc.
Note: Macro expands to {} if not configured, so no #ifdef is needed.
Comment:
Signed-off-by: Christian Storm christian.storm@tngtech.com Signed-off-by: Jan Kiszka jan.kiszka@siemens.com Signed-off-by: Andreas J. Reichel Andreas.Reichel@tngtech.com
Thanks for the detailed info.
Would it be better to put this in _serial_tstc()?
common/console.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/common/console.c b/common/console.c index 12293f3..054b6cd 100644 --- a/common/console.c +++ b/common/console.c @@ -16,6 +16,7 @@ #include <stdio_dev.h> #include <exports.h> #include <environment.h> +#include <watchdog.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -294,6 +295,7 @@ int fgetc(int file) * Effectively poll for input wherever it may be available. */ for (;;) {
WATCHDOG_RESET(); /* * Upper layer may have already called tstc() so * check for that first.-- 2.8.2
Regards, Simon
This patch fixes unwanted watchdog resets while the user enters a command at the U-Boot prompt.
As found on the CM-FX6 board from Compulab, when having enabled the watchdog, a missing WATCHDOG_RESET call in _serial_tstc() in (/drivers/serial/serial-uclass.c) causes this and alike boards to reset when the watchdog's timeout has elapsed while waiting at the U-Boot prompt.
Despite the user could press several keys within the watchdog timeout limit, the while loop in cli_readline.c, line 261, does only call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st loop iteration. This leads to a watchdog timeout no matter if the user presses keys or not.
The problem is solved with a call to WATCHDOG_RESET in _serial_tstc, defined in drivers/serial/serial-uclass.c.
Since the macro WATCHDOG_RESET expands to {} if watchdog support isn't configured, there's no need to surround it by #ifdef in this case.
* Symptom: U-Boot resets after watchdog times out when in commandline prompt and watchdog is enabled.
* Reasoning: When U-Boot shows the commandline prompt, the following function call stack is executed while waiting for a keypress:
common/main.c: main_loop => common/cli.c: cli_loop() => common/cli_hush.c: parse_file_outer => parse_stream_outer => parse_stream => b_getch(i) => i->get(i) => file_get => get_user_input => cmdedit_read_input => uboot_cli_readline => common/cli_readline.c: cli_readline => cli_readline_into_buffer => cread_line => getcmd_getch (== getc) => commonn/console.c: fgetc => console_tstc
- in console_tstc line 181: If dev->tstc(dev) returns 0, the global tstcdev variable doesn't get set. This is the case if no character is in the serial buffer.
- in fgetc(int file), line 297: Program flow keeps looping because tstcdev does not get set. Therefore WATCHDOG_RESET is not called, as mx_serial_tstc from drivers/serial/serial_mxc.c does not call it.
- Initialization calls drv_system_init in stdio.c, which sets dev.tstc = stdio_serial_tstc. Thus, dev->tstc(dev) calls serial_tstc() which in turn calls _serial_tstc().
Hence, _serial_tstc() needs to call WATCHDOG_RESET() to periodically reset the watchdog while cli_readline waits for user input.
Signed-off-by: Christian Storm christian.storm@tngtech.com Signed-off-by: Jan Kiszka jan.kiszka@siemens.com Signed-off-by: Andreas J. Reichel Andreas.Reichel@tngtech.com ---
Changes in v2: - Move WATCHDOG_RESET() call from common/console.c to drivers/serial/serial-uclass.c.
drivers/serial/serial-uclass.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index 0ce5c44..72cf808 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -155,6 +155,7 @@ static int _serial_tstc(struct udevice *dev) { struct dm_serial_ops *ops = serial_get_ops(dev);
+ WATCHDOG_RESET(); if (ops->pending) return ops->pending(dev, true);
+Tom, in case this should go into the release.
On 1 August 2016 at 05:49, Andreas J. Reichel Andreas.Reichel@tngtech.com wrote:
This patch fixes unwanted watchdog resets while the user enters a command at the U-Boot prompt.
As found on the CM-FX6 board from Compulab, when having enabled the watchdog, a missing WATCHDOG_RESET call in _serial_tstc() in (/drivers/serial/serial-uclass.c) causes this and alike boards to reset when the watchdog's timeout has elapsed while waiting at the U-Boot prompt.
Despite the user could press several keys within the watchdog timeout limit, the while loop in cli_readline.c, line 261, does only call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st loop iteration. This leads to a watchdog timeout no matter if the user presses keys or not.
The problem is solved with a call to WATCHDOG_RESET in _serial_tstc, defined in drivers/serial/serial-uclass.c.
Since the macro WATCHDOG_RESET expands to {} if watchdog support isn't configured, there's no need to surround it by #ifdef in this case.
Symptom: U-Boot resets after watchdog times out when in commandline prompt and watchdog is enabled.
Reasoning: When U-Boot shows the commandline prompt, the following function call stack is executed while waiting for a keypress:
common/main.c: main_loop => common/cli.c: cli_loop() => common/cli_hush.c: parse_file_outer => parse_stream_outer => parse_stream => b_getch(i) => i->get(i) => file_get => get_user_input => cmdedit_read_input => uboot_cli_readline => common/cli_readline.c: cli_readline => cli_readline_into_buffer => cread_line => getcmd_getch (== getc) => commonn/console.c: fgetc => console_tstc
- in console_tstc line 181:
If dev->tstc(dev) returns 0, the global tstcdev variable doesn't get set. This is the case if no character is in the serial buffer.
- in fgetc(int file), line 297:
Program flow keeps looping because tstcdev does not get set. Therefore WATCHDOG_RESET is not called, as mx_serial_tstc from drivers/serial/serial_mxc.c does not call it.
- Initialization calls drv_system_init in stdio.c, which sets
dev.tstc = stdio_serial_tstc. Thus, dev->tstc(dev) calls serial_tstc() which in turn calls _serial_tstc().
Hence, _serial_tstc() needs to call WATCHDOG_RESET() to periodically reset the watchdog while cli_readline waits for user input.
Signed-off-by: Christian Storm christian.storm@tngtech.com Signed-off-by: Jan Kiszka jan.kiszka@siemens.com Signed-off-by: Andreas J. Reichel Andreas.Reichel@tngtech.com
Changes in v2:
- Move WATCHDOG_RESET() call from common/console.c to drivers/serial/serial-uclass.c.
drivers/serial/serial-uclass.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index 0ce5c44..72cf808 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -155,6 +155,7 @@ static int _serial_tstc(struct udevice *dev) { struct dm_serial_ops *ops = serial_get_ops(dev);
WATCHDOG_RESET(); if (ops->pending) return ops->pending(dev, true);
Great explanation, thank you.
Acked-by: Simon Glass sjg@chromium.org
-- 2.8.2
Hi Tom,
is my patch going to be applied or is the problem resolved otherwhise?
Kind regards Andreas
On Mo, Sep 05, 2016 at 07:04:53 -0600, Simon Glass wrote:
+Tom, in case this should go into the release.
On 1 August 2016 at 05:49, Andreas J. Reichel Andreas.Reichel@tngtech.com wrote:
This patch fixes unwanted watchdog resets while the user enters a command at the U-Boot prompt.
As found on the CM-FX6 board from Compulab, when having enabled the watchdog, a missing WATCHDOG_RESET call in _serial_tstc() in (/drivers/serial/serial-uclass.c) causes this and alike boards to reset when the watchdog's timeout has elapsed while waiting at the U-Boot prompt.
Despite the user could press several keys within the watchdog timeout limit, the while loop in cli_readline.c, line 261, does only call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st loop iteration. This leads to a watchdog timeout no matter if the user presses keys or not.
The problem is solved with a call to WATCHDOG_RESET in _serial_tstc, defined in drivers/serial/serial-uclass.c.
Since the macro WATCHDOG_RESET expands to {} if watchdog support isn't configured, there's no need to surround it by #ifdef in this case.
Symptom: U-Boot resets after watchdog times out when in commandline prompt and watchdog is enabled.
Reasoning: When U-Boot shows the commandline prompt, the following function call stack is executed while waiting for a keypress:
common/main.c: main_loop => common/cli.c: cli_loop() => common/cli_hush.c: parse_file_outer => parse_stream_outer => parse_stream => b_getch(i) => i->get(i) => file_get => get_user_input => cmdedit_read_input => uboot_cli_readline => common/cli_readline.c: cli_readline => cli_readline_into_buffer => cread_line => getcmd_getch (== getc) => commonn/console.c: fgetc => console_tstc
- in console_tstc line 181:
If dev->tstc(dev) returns 0, the global tstcdev variable doesn't get set. This is the case if no character is in the serial buffer.
- in fgetc(int file), line 297:
Program flow keeps looping because tstcdev does not get set. Therefore WATCHDOG_RESET is not called, as mx_serial_tstc from drivers/serial/serial_mxc.c does not call it.
- Initialization calls drv_system_init in stdio.c, which sets
dev.tstc = stdio_serial_tstc. Thus, dev->tstc(dev) calls serial_tstc() which in turn calls _serial_tstc().
Hence, _serial_tstc() needs to call WATCHDOG_RESET() to periodically reset the watchdog while cli_readline waits for user input.
Signed-off-by: Christian Storm christian.storm@tngtech.com Signed-off-by: Jan Kiszka jan.kiszka@siemens.com Signed-off-by: Andreas J. Reichel Andreas.Reichel@tngtech.com
Changes in v2:
- Move WATCHDOG_RESET() call from common/console.c to drivers/serial/serial-uclass.c.
drivers/serial/serial-uclass.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index 0ce5c44..72cf808 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -155,6 +155,7 @@ static int _serial_tstc(struct udevice *dev) { struct dm_serial_ops *ops = serial_get_ops(dev);
WATCHDOG_RESET(); if (ops->pending) return ops->pending(dev, true);Great explanation, thank you.
Acked-by: Simon Glass sjg@chromium.org
-- 2.8.2
On Mon, Sep 19, 2016 at 01:59:44PM +0200, Andreas Reichel wrote:
Hi Tom,
is my patch going to be applied or is the problem resolved otherwhise?
Sorry, lost in my patchwork queue. I'll pick this up soon now.
Kind regards Andreas
On Mo, Sep 05, 2016 at 07:04:53 -0600, Simon Glass wrote:
+Tom, in case this should go into the release.
On 1 August 2016 at 05:49, Andreas J. Reichel Andreas.Reichel@tngtech.com wrote:
This patch fixes unwanted watchdog resets while the user enters a command at the U-Boot prompt.
As found on the CM-FX6 board from Compulab, when having enabled the watchdog, a missing WATCHDOG_RESET call in _serial_tstc() in (/drivers/serial/serial-uclass.c) causes this and alike boards to reset when the watchdog's timeout has elapsed while waiting at the U-Boot prompt.
Despite the user could press several keys within the watchdog timeout limit, the while loop in cli_readline.c, line 261, does only call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st loop iteration. This leads to a watchdog timeout no matter if the user presses keys or not.
The problem is solved with a call to WATCHDOG_RESET in _serial_tstc, defined in drivers/serial/serial-uclass.c.
Since the macro WATCHDOG_RESET expands to {} if watchdog support isn't configured, there's no need to surround it by #ifdef in this case.
Symptom: U-Boot resets after watchdog times out when in commandline prompt and watchdog is enabled.
Reasoning: When U-Boot shows the commandline prompt, the following function call stack is executed while waiting for a keypress:
common/main.c: main_loop => common/cli.c: cli_loop() => common/cli_hush.c: parse_file_outer => parse_stream_outer => parse_stream => b_getch(i) => i->get(i) => file_get => get_user_input => cmdedit_read_input => uboot_cli_readline => common/cli_readline.c: cli_readline => cli_readline_into_buffer => cread_line => getcmd_getch (== getc) => commonn/console.c: fgetc => console_tstc
- in console_tstc line 181:
If dev->tstc(dev) returns 0, the global tstcdev variable doesn't get set. This is the case if no character is in the serial buffer.
- in fgetc(int file), line 297:
Program flow keeps looping because tstcdev does not get set. Therefore WATCHDOG_RESET is not called, as mx_serial_tstc from drivers/serial/serial_mxc.c does not call it.
- Initialization calls drv_system_init in stdio.c, which sets
dev.tstc = stdio_serial_tstc. Thus, dev->tstc(dev) calls serial_tstc() which in turn calls _serial_tstc().
Hence, _serial_tstc() needs to call WATCHDOG_RESET() to periodically reset the watchdog while cli_readline waits for user input.
Signed-off-by: Christian Storm christian.storm@tngtech.com Signed-off-by: Jan Kiszka jan.kiszka@siemens.com Signed-off-by: Andreas J. Reichel Andreas.Reichel@tngtech.com
Changes in v2:
- Move WATCHDOG_RESET() call from common/console.c to drivers/serial/serial-uclass.c.
drivers/serial/serial-uclass.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c index 0ce5c44..72cf808 100644 --- a/drivers/serial/serial-uclass.c +++ b/drivers/serial/serial-uclass.c @@ -155,6 +155,7 @@ static int _serial_tstc(struct udevice *dev) { struct dm_serial_ops *ops = serial_get_ops(dev);
WATCHDOG_RESET(); if (ops->pending) return ops->pending(dev, true);Great explanation, thank you.
Acked-by: Simon Glass sjg@chromium.org
-- 2.8.2
-- Andreas Reichel Dipl.-Phys. (Univ.) Software Consultant
Andreas.Reichel@tngtech.com +49-174-3180074
TNG Technology Consulting GmbH, Betastr. 13a, 85774 Unterföhring Geschäftsführer: Henrik Klagges, Christoph Stock, Dr. Robert Dahlke Sitz: Unterföhring * Amtsgericht München * HRB 135082
This patch fixes unwanted watchdog resets while the user enters a command at the U-Boot prompt.
As found on the CM-FX6 board from Compulab, when having enabled the watchdog, a missing WATCHDOG_RESET call in _serial_tstc() in (/drivers/serial/serial-uclass.c) causes this and alike boards to reset when the watchdog's timeout has elapsed while waiting at the U-Boot prompt.
Despite the user could press several keys within the watchdog timeout limit, the while loop in cli_readline.c, line 261, does only call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st loop iteration. This leads to a watchdog timeout no matter if the user presses keys or not.
The problem is solved with a call to WATCHDOG_RESET in _serial_tstc, defined in drivers/serial/serial-uclass.c.
Since the macro WATCHDOG_RESET expands to {} if watchdog support isn't configured, there's no need to surround it by #ifdef in this case.
Changes in v2: - Move WATCHDOG_RESET() call from common/console.c to drivers/serial/serial-uclass.c.
Andreas J. Reichel (1): watchdog: Fix Watchdog Reset while in U-Boot Prompt
drivers/serial/serial-uclass.c | 1 + 1 file changed, 1 insertion(+)
On Wed, Jul 13, 2016 at 12:56:51PM +0200, Andreas J. Reichel wrote:
Hardware: CM-FX6 Module from Compulab
This patch fixes unwanted watchdog resets while the user enters a command at the U-Boot prompt.
As found on the CM-FX6 board from Compulab, when having enabled the watchdog, a missing WATCHDOG_RESET call in common/console.c causes this and alike boards to reset when the watchdog's timeout has elapsed while waiting at the U-Boot prompt.
Despite the user could press several keys within the watchdog timeout limit, the while loop in cli_readline.c, line 261, does only call WATCHDOG_RESET if first == 1, which gets set to 0 in the 1st loop iteration. This leads to a watchdog timeout no matter if the user presses keys or not.
Although, this affects other boards as well as it touches common/console.c, the macro WATCHDOG_RESET expands to {} if watchdog support isn't configured. Hence, there's no harm caused and no need to surround it by #ifdef in this case.
Symptom: U-Boot resets after watchdog times out when in commandline prompt and watchdog is enabled.
Reasoning: When U-Boot shows the commandline prompt, the following function call stack is executed while waiting for a keypress:
common/main.c: main_loop => common/cli.c: cli_loop() => common/cli_hush.c: parse_file_outer => parse_stream_outer => parse_stream => b_getch(i) => i->get(i) => file_get => get_user_input => cmdedit_read_input => uboot_cli_readline => common/cli_readline.c: cli_readline => cli_readline_into_buffer => cread_line => getcmd_getch (== getc) => common/console.c: fgetc => console_tstc
common/console.c: (with CONFIG_CONSOLE_MUX is set)
- in console_tstc line 181:
If dev->tstc(dev) returns 0, the global tstcdev variable doesn't get set. This is the case if no character is in the serial buffer.
- in fgetc(int file), line 297:
Program flow keeps looping because tstcdev does not get set. Therefore WATCHDOG_RESET is not called, as mx_serial_tstc from drivers/serial/serial_mxc.c does not call it.
Solution: Add WATCHDOG_RESET into the loop of console_tstc.
Note: Macro expands to {} if not configured, so no #ifdef is needed.
Comment:
Signed-off-by: Christian Storm christian.storm@tngtech.com Signed-off-by: Jan Kiszka jan.kiszka@siemens.com Signed-off-by: Andreas J. Reichel Andreas.Reichel@tngtech.com Acked-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (4)
-
Andreas J. Reichel -
Andreas Reichel
-
Simon Glass -
Tom Rini