Hi Manfred,
On Thu, Mar 21, 2013 at 8:03 PM, Manfred Huber man.huber@arcor.de wrote:
From: Manfred Huber
Beagleboard UART (ns16550) doesn't set the Transmitter Empty (TEMT) Bit in SPL. Only Transmitter Hold Register Empty (THRE) Bit is set. This makes SPL to hang while waiting for TEMT. Adding the CONFIG_SYS_NS16550_BROKEN_TEMT config option and waiting for THRE avoid this issue.
Signed-off-by: Manfred Huber man.huber@arcor.de
drivers/serial/ns16550.c | 5 ++++- include/configs/omap3_beagle.h | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index b2da8b3..6379bcc 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -36,7 +36,10 @@
void NS16550_init(NS16550_t com_port, int baud_divisor) { -#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT)) +#if defined(CONFIG_SPL_BUILD) && defined(CONFIG_SYS_NS16550_BROKEN_TEMT)
while (!(serial_in(&com_port->lsr) & UART_LSR_THRE));+#else
I'm not sure to add this behavior under CONFIG_SYS_NS16550_BROKEN_TEMT since this option just means:
"On some broken platforms the Transmitter Empty (TEMT) bit is not set in SPL making U-Boot to hang while waiting for it"
According to your findings it seems that some OMAP3 platforms (at least DM3730 and 3530) set THRE but I wonder if other broken platforms will behave the same.
The current CONFIG_SYS_NS16550_BROKEN_TEMT just skips this test so it can be reused by other platforms, but now your change makes it less generic since it will only work on platforms that set THRE.
So I would just leave CONFIG_SYS_NS16550_BROKEN_TEMT as is now and be able to reuse it on other platforms or add a new config option CONFIG_SYS_NS16550_WAIT_THRE or something that would test for THRE instead of TEMT and use that for OMAP3 based boards.
I do like your change that adds a test for CONFIG_SPL_BUILD since even in the README it says that the issue is only present in SPL. So I just forgot to add this when added the config option.
while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT)) ;#endif diff --git a/include/configs/omap3_beagle.h b/include/configs/omap3_beagle.h index 48ce4c0..6ab46d5 100644 --- a/include/configs/omap3_beagle.h +++ b/include/configs/omap3_beagle.h @@ -82,6 +82,9 @@ #define CONFIG_SYS_NS16550_REG_SIZE (-4) #define CONFIG_SYS_NS16550_CLK V_NS16550_CLK
+/* define to avoid U-Boot to hang while waiting for TEMT */ +#define CONFIG_SYS_NS16550_BROKEN_TEMT
/*
- select serial console configuration
*/
This part of your patch looks good to me but you should split your changes in two patches. One to change the ns16550 drvier and another one to add this config option for the Beagleboard.
Thanks a lot and best regards, Javier