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