On 08/10/2018 07:22 AM, Simon Goldschmidt wrote: > On 10.08.2018 00:41, Marek Vasut wrote: >> On 08/10/2018 12:35 AM, Andy Shevchenko wrote: >>> On Fri, Aug 10, 2018 at 12:45 AM, Marek Vasut <ma...@denx.de> wrote: >>>> On 08/09/2018 11:13 PM, Adam Ford wrote: >>>>> On Thu, Aug 9, 2018 at 2:08 PM Simon Goldschmidt >>>>> <simon.k.r.goldschm...@gmail.com> 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? >> But why do we care about the divisor at all ? > > Because if the divisor is zero, the UART is disabled. > >> The real problem I believe >> is that someone can call debug UART print/read functions before it is >> inited. >> > I know this is a hack. I did it like that because I need something like > this to get debug uart to work on socfpga gen5 (there always is a printf > before debug uart init is possible). > > A generic solution for all debug uarts would be better of course, but > given the point in SPL runtime, we might have to add a field to 'gd' for > that, or does a global variable work at that point already?
GD field might be needed indeed. -- Best regards, Marek Vasut _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot