[U-Boot] [PATCH v2] ns16550: Add WATCHDOG_RESET to putc for short watchdog timeout boards

[Wolfgang Denk <wd@denx.de>]

Dear Stefan Roese,

In message <1286434900-11804-1-git-send-email-sr@denx.de> you 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?

>  #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.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
"Never face facts; if you do, you'll never get up in the morning."
- Marlo Thomas