On 2013-03-27 14:37, Andreas Bießmann wrote:
Dear Manfred Huber,
---8<---
abiessmann@punisher % pwclient get 230994
Saved patch to
U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch
abiessmann@punisher % git am
U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch
Patch does not have a valid e-mail address.
abiessmann@punisher % ./tools/checkpatch.pl
U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch
ERROR: trailing whitespace
#39: FILE: drivers/serial/ns16550.c:40:
+^Iif ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE)) == $
ERROR: patch seems to be corrupt (line wrapped?)
#40: FILE: drivers/serial/ns16550.c:40:
UART_LSR_THRE) {
total: 2 errors, 0 warnings, 20 lines checked
NOTE: whitespace errors detected, you may wish to use scripts/cleanpatch or
scripts/cleanfile
NOTE: Ignored message types: COMPLEX_MACRO CONSIDER_KSTRTO MINMAX
MULTISTATEMENT_MACRO_USE_DO_WHILE
U-Boot-1-1-v2-omap3_beagle-Enabling-UART3-first-allows-the-Transmitter-to-be-empty.patch
has style problems, please review.
If any of these errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
--->8---
Can you please fix these errors?
I will do it.
On 03/25/2013 11:02 PM, Manfred Huber wrote:
From: Manfred Huber
Due to a Bug in the ROM code of some OMAP3 devices, the TEMT bit is not
set if UART3 is configured before (only THRE is set). Reason is the
disabling of UART3 even though the Transmitter is not empty. Enabling
UART3 allows the Transmitter to be empty.
Signed-off-by: Manfred Huber <man.hu...@arcor.de>
---
drivers/serial/ns16550.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index b2da8b3..24ff84f 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -36,10 +36,18 @@
void NS16550_init(NS16550_t com_port, int baud_divisor)
{
-#if (!defined(CONFIG_SYS_NS16550_BROKEN_TEMT))
+#if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX))
I think a comment here would be good.
I will do it.
+ if ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE))
== UART_LSR_THRE) {
The patch is broken here, even if you fix the wrapping you will get an
'over 80 char' error here.
+ serial_out(UART_LCR_DLAB, &com_port->lcr);
+ serial_out(baud_divisor & 0xff, &com_port->dll);
+ serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);
+ serial_out(UART_LCRVAL, &com_port->lcr);
+ serial_out(0, &com_port->mdr1);
Do we need to setup baud a.s.o. here? Isn't it enough to issue an soft
reset of the UART? I'm not in this material, I just wonder if we can
omit some of the lines here cause we set e.g. the BAUD later on.
The reason to setup the baud is for the shift register. It only works
with programmed baud registers. A soft reset would also work, but as
Scott Wood said it would corrupt the last character. On the other hand
the character should be corrupted by disabling the UART. I have no
preferred solution: programming the UART or a soft reset. Maybe someone
wants to decide.
+ }
+#endif
+
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)) || \
I managed to apply your patch anyhow. A short test on a tricorder board
showed no harm to the boot process. So please get your patch clean and
resend, then I will add my tested-by.
As Javier pointed out please think about using the
CONFIG_SYS_NS16550_BROKEN_TEMT instead of SPL && OMAP34XX.
Another solution could be to have this TEMT | THRE check in
unconditionally, this however would require a lot more testing.
Especially with the release date in mind.
It's not critical. So I guess it's not needed for this release.
Best regards
Andreas Bießmann
Best regards,
Manfred
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
