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