On 03/08/2015 05:04 PM, Måns Rullgård wrote: > Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> writes: > >> Au1x00/RT2800+ doesn't implement the 8250 scratch register (and >> this may be true of other h/w currently supported by the 8250 driver); >> read back the canary value written to the scratch register to enable >> the console h/w restart after resume from system suspend. >> >> Fixes: 4516d50aabedb ("serial: 8250: Use canary to restart console ...") >> Reported-by: Mason <slash.tmp@xxxxxxx> >> Signed-off-by: Peter Hurley <peter@xxxxxxxxxxxxxxxxxx> >> --- >> drivers/tty/serial/8250/8250_core.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c >> index 87df908..50c80b1 100644 >> --- a/drivers/tty/serial/8250/8250_core.c >> +++ b/drivers/tty/serial/8250/8250_core.c >> @@ -3486,7 +3486,8 @@ void serial8250_suspend_port(int line) >> port->type != PORT_8250) { >> unsigned char canary = 0xa5; >> serial_out(up, UART_SCR, canary); >> - up->canary = canary; >> + if (serial_in(up, UART_SCR) == canary) >> + up->canary = canary; >> } > > I think this has the opposite of the intended effect. If there is no > scratch register, up->canary will remain zero, and the reset code in > serial8250_console_write() won't execute even if it is needed. That was intended. Case #1 - UART has UART_SCR The canary written is read back so up->canary is set to non-zero. At each subsequent write to console, the scratch register will be read until the canary has changed, at which time the UART will be reinitialized and the canary test disabled (on the presumption that what changed the canary was a power-off). Note that if the port does not power-down at system suspend (eg, if system suspend failed), the canary test is disabled anyway at port resume. Case #2 - UART does not have UART_SCR The canary written is not read back properly so up->canary remains zero. No attempt is made to test the canary or reinitialize the UART. This leaves the uart in the same state it would have been in; the canary test is useless here because there continues to be output after port suspend but before machine suspend. > As it is currently (without this patch), a missing scratch register will > only result in the port possibly being re-initialised unnecessarily, > which should be quite harmless. I'd rather not trigger the reinitialize if it's not necessary; does it do anything on your hardware (that doesn't have UART_SCR) that's actually useful? 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