On 01/27/2015 09:05 AM, Rob Herring wrote: > On Tue, Jan 27, 2015 at 7:10 AM, Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> wrote: >> On 01/26/2015 11:50 PM, Rob Herring wrote: >>> The PXA variant of the 8250 UART adds a UART enable bit which must not >>> be cleared. Make the earlycon support maintain this bit if it is set. >>> This implies some initialization of the UART, but we cannot >>> unconditionally set the bit as some other variants require this bit to >>> be clear for other functions. >> >> So if the bootloader doesn't enable the uart, then earlycon doesn't work? > > Correct. Some earlycon drivers rely on this fact and don't do any > initialization. There is a patch posted also to skip init for the 8250 > earlycon. We could rely on that instead, but the 1st chunk is still > needed. > > The UART already being initialized should be pretty common for PXA > based systems. More so than x86 UARTs. I think it is better to work > under some conditions than none. Ok. >> Or is there some other piece that is enabling the uart after earlycon setup >> (besides the driver, of course)? >> >>> Signed-off-by: Rob Herring <robh@xxxxxxxxxx> >>> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> >>> Cc: Jiri Slaby <jslaby@xxxxxxx> >>> Cc: linux-serial@xxxxxxxxxxxxxxx >>> --- >>> drivers/tty/serial/8250/8250_early.c | 8 +++++--- >>> 1 file changed, 5 insertions(+), 3 deletions(-) >>> >>> diff --git a/drivers/tty/serial/8250/8250_early.c b/drivers/tty/serial/8250/8250_early.c >>> index ce2a8ab..c31a22b 100644 >>> --- a/drivers/tty/serial/8250/8250_early.c >>> +++ b/drivers/tty/serial/8250/8250_early.c >>> @@ -93,10 +93,10 @@ static void __init early_serial8250_write(struct console *console, >>> struct uart_port *port = &early_device->port; >>> unsigned int ier; >>> >>> - /* Save the IER and disable interrupts */ >>> + /* Save the IER and disable interrupts preserving the UUE bit */ >>> ier = serial8250_early_in(port, UART_IER); >>> if (ier) >>> - serial8250_early_out(port, UART_IER, 0); >>> + serial8250_early_out(port, UART_IER, ier & UART_IER_UUE); >>> >>> uart_console_write(port, s, count, serial_putc); >>> >>> @@ -127,9 +127,11 @@ static void __init init_port(struct earlycon_device *device) >>> struct uart_port *port = &device->port; >>> unsigned int divisor; >>> unsigned char c; >>> + unsigned int ier; >>> >>> serial8250_early_out(port, UART_LCR, 0x3); /* 8n1 */ >>> - serial8250_early_out(port, UART_IER, 0); /* no interrupt */ >>> + ier = serial8250_early_in(port, UART_IER); >>> + serial8250_early_out(port, UART_IER, ier & UART_IER_UUE); /* no interrupt */ >> >> The trailing comment seems no longer relevant; how about below? > > It is because we are still disabling interrupts. I realize that, which is why my suggested replacement comment includes that information. It's not obvious what effect UART_IER_UUE has from just looking at the code. >> /* Mask interrupts (preserve XScale's uart enable bit) */ >> serial8250_early_out(port, UART_IER, ier & UART_IER_UUE); >> >> OTOH, if the uart is disabled, shouldn't earlycon be disabled? > > There's no way I can detect that. Right, sorry. [ I was about to write, "If UART_IER_UEE is not set, ..." which of course is true for every other non-PXA 8250-base uart :) ] Regards, Peter Hurley -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
