Hi Scott, thank you for your feedback.
On 12/22/2012 03:45 AM, Scott Wood wrote:
> On 12/21/2012 03:21:46 AM, Javier Martinez Canillas wrote:
>> On ns16550, the Transmitter Empty (TEMT) Bit is used to
>> indicate when the Transmitter Holding Register (THR) and
>> the Transmitter Shift Register (TSR) are both empty.
>>
>> But ns16550 UART has two operation modes (16450 and FIFO)
>> and the TEMT bit logic value set is different on each mode.
>>
>> On 16450, the TEMT bit is set to 1 when both THR and TSR are
>> empty and is set to 0 on FIFO mode.
>
> When you say "on 16450", do you mean "in 16450 mode"?
>
Yes, that's what I meant. I'm not a native English speaker so sorry if I have
some grammatic errors in my comments.
>> So, checking the TEMT value without checking the current mode
>> and assuming a logical value of 1, can lead to U-Boot to hang
>> forever if the UART is initialized on FIFO mode by default.
>
> From the 16550 docs:
>
> Bit 6 This bit is the Transmitter Empty (TEMT) indicator Bit 6 is set
> to a logic 1 whenever the Transmitter Holding Regis- ter (THR) and
> the
> Transmitter Shift Register (TSR) are both empty It is reset to a
> logic
> 0 whenever either the THR or TSR contains a data character In the
> FIFO
> mode this bit is set to one whenever the transmitter FIFO and shift
> register are both empty
>
> Maybe the 16550 implementation you're using is doing something wrong?
>
Could be, I'm using a board with an TI OMAP3 SoC which has an NS16650 UART.
Now I read again the NS16650 documentation [1] and you are right. I
misunderstood and thought that on FIFO mode the TEMT bit was set to 0 when the
both THR and TSR were empty and that this was causing U-Boot to hang.
BTW I realized that this is only an issue for SPL, if you build that check for
the non-SPL build, it works.
With this patch my board boots:
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index bbd91ca..a3ef8a5 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -36,8 +36,10 @@
void NS16550_init(NS16550_t com_port, int baud_divisor)
{
+#ifndef CONFIG_SPL_BUILD
while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT))
;
+#endif
serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier);
#if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \
>> Signed-off-by: Javier Martinez Canillas
>> <javier.marti...@collabora.co.uk>
>> ---
>> drivers/serial/ns16550.c | 4 +++-
>> 1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
>> index bbd91ca..d75d814 100644
>> --- a/drivers/serial/ns16550.c
>> +++ b/drivers/serial/ns16550.c
>> @@ -36,7 +36,9 @@
>>
>> void NS16550_init(NS16550_t com_port, int baud_divisor)
>> {
>> - while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT))
>> + int mode = serial_in(&com_port->fcr) & UART_FCR_FIFO_EN;
>> +
>> + while (!(serial_in(&com_port->lsr) & UART_LSR_TEMT ^ mode))
>> ;
>
> Please don't make the reader know the relative prededence of & and ^,
> and
> don't use ^ in obfuscatory ways. It's not even any different from | as
> used above -- except that | wouldn't break if TEMT and FIFO_EN had the
> same
> value -- so why do you need the exclusive form?
>
Ok, sorry. I didn't think that knowing the precedence of bitwise operator was
obfuscated and the ^ was because I misunderstood the NS16550 docs and thought
that UART_LSR_TEMT would be 0 in FIFO mode.
> BTW, when I saw the problem that necessitated this, FIFO was enabled, so
> this is no better than reverting the patch.
>
> -Scott
>
Either way works for me, I just want to boot my board :-)
But if I'm the only one having this issue maybe is just my hardware behaving
badly. I'll ask other OMAP3 users if they can boot with mainline U-Boot to
confirm this.
Best regards,
Javier
[1]:http://www.ti.com/lit/ds/symlink/pc16550d.pdf
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
