From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Shevchenko Date: Fri, 10 Aug 2018 01:35:53 +0300 Subject: [U-Boot] [PATCH 5/6] serial: ns16550: fix debug uart putc called before init In-Reply-To: <95af5965-0755-415a-db58-2b9863c30d7d@denx.de> References: <20180805193500.3693-1-simon.k.r.goldschmidt@gmail.com> <20180809190420.28093-1-simon.k.r.goldschmidt@gmail.com> <20180809190420.28093-6-simon.k.r.goldschmidt@gmail.com> <95af5965-0755-415a-db58-2b9863c30d7d@denx.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Fri, Aug 10, 2018 at 12:45 AM, Marek Vasut wrote: > On 08/09/2018 11:13 PM, Adam Ford wrote: >> On Thu, Aug 9, 2018 at 2:08 PM Simon Goldschmidt >> wrote: >>> >>> If _debug_uart_putc() is called before _debug_uart_init(), the >>> ns16550 debug uart driver hangs in a tight loop waiting for the >>> tx FIFO to get empty. >>> >>> As this can happen via a printf sneaking in before the port calls >>> debug_uart_init(), let's rather ignore characters before the debug >>> uart is initialized. >>> >>> This is done by reading the baudrate divisor and aborting if is zero. >>> static inline void _debug_uart_putc(int ch) >>> { >>> struct NS16550 *com_port = (struct NS16550 *)CONFIG_DEBUG_UART_BASE; >>> + while (!(serial_din(&com_port->lsr) & UART_LSR_THRE)) { >>> + if (!NS16550_read_baud_divisor(com_port)) >> >> Unless there is a change that the read_baud_divisor will change while >> we're waiting for the character, could we move this check before the >> while statement? This would reduce the check for the divisor to 1x >> and the while statement would only have one comparison to do. I >> realize it's rather trivial, but the way I see it, there is no reason >> to do the while statement at all if the read_baud_divisor fails and >> there if there is a baud divisor, we should only need to check it >> once. > > This looks like a massive hack -- what about having a flag which says > that the debug uart was/was not inited somewhere ? Agree, why not to cache divisor value, for example, instead of doing slow I/O? -- With Best Regards, Andy Shevchenko