On Wed, Mar 27, 2013 at 2:37 PM, Andreas Bießmann <andreas.de...@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.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. > >> + 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 _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot