Re: [PATCH 2/2] serial: 8250: Check UART_SCR is writable

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux PPP]     [Linux FS]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Linmodem]     [Device Mapper]     [Linux Kernel for ARM]

  Powered by Linux