On Wed, Mar 27, 2013 at 2:37 PM, Andreas Bießmann andreas.devel@googlemail.com 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?
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.huber@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.
- 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.
- }
+#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.
Best regards
Andreas Bießmann
Hi Manfred,
I just tested your patch and my IGEPv2 boots correctly.
So, after addressing the issues pointed out by Andreas feel free to add my Tested-by too.
Thanks a lot and best regards, Javier