Hi Wolfgang,
On Thursday 07 October 2010 12:17:33 Wolfgang Denk wrote:
This is needed for board with a very short watchdog timeout, like the lwmon5 with a 100ms timeout. Without this patch this board resets in the commands with long outputs, like "printenv" or "fdt print".
Sorry, but I still think there must be something awfully wrong with that code.
#include <config.h>
+#include <common.h>
Why do you need this new include?
I added this while testing with udelay (see below). You're right, it's not needed any more.
#include <ns16550.h> #include <watchdog.h> #include <linux/types.h>
@@ -68,7 +69,13 @@ void NS16550_reinit (NS16550_t com_port, int baud_divisor)
void NS16550_putc (NS16550_t com_port, char c) {
- while ((serial_in(&com_port->lsr) & UART_LSR_THRE) == 0);
- int count = 0;
- while ((serial_in(&com_port->lsr) & UART_LSR_THRE) == 0) {
/* reset watchdog from time to time */if ((count++ % (256 << 10)) == 0)WATCHDOG_RESET();- }
We are in the putc() routine here, i. e. when printing a single character. This is a place which never can take 100 milliseconds, so it cannot be the culprit for any watchdog timeouts.
And you reinitialize the counter for each and every character printed. That means that your choice of the loop limit (256 << 10) must short enough that it gets triggered for essentially _all_ invocations of putc(), which is definitely overkill for triggering the watchdog.
I thing this is the wrong place to address the problem.
The idea of implementing the watchdog_reset call in this function comes from the original implementation in the PPC4xx UART driver. As the ns166550 driver is now used on PPC4xx, this is missing right now and results in resets upon long outputs (e.g. printenv, fdt print) on lwmon5. The "old" PPC4xx UART driver did call udelay() in the putc() function. This implicitly called WATCHDOG_RESET(). I didn't want to add this udelay to ns16550, since this would lead to code increase for platforms not using this watchdog.
Back to the problem: How about moving the watchdog_reset call to serial_puts() in common/serial.c? Or do you have another (better) idea in mind?
Cheers, Stefan
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office@denx.de