Re: [PATCH] Do not read IIR in serial8250_start_tx when UART_BUG_TXEN

[Jiri Kosina <jkosina@suse.cz>]

On Wed, 18 Nov 2009, Jiri Kosina wrote:

> The patch below has been lost several times (the last submission happened 
> probably on [1]). I am currently aware of a real hardware which triggers 
> this problem quite reliably, so we should rather have that really fixed.
> 
> [1] http://lkml.org/lkml/2009/3/12/379
> 
> 
> 
> 
> From: Ian Jackson <ian.jackson@eu.citrix.com>
> 
> Do not read IIR in serial8250_start_tx when UART_BUG_TXEN
> 
> Reading the IIR clears some oustanding interrupts so it is not safe.
> Instead, simply transmit immediately if the buffer is empty without
> regard to IIR.
> 
> Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Jiri Kosina <jkosina@suse.cz>
> 
> ---
>  drivers/serial/8250.c |    8 +++-----
>  1 files changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> index 737b4c9..807042b 100644
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -1339,14 +1339,12 @@ static void serial8250_start_tx(struct uart_port *port)
>  		serial_out(up, UART_IER, up->ier);
>  
>  		if (up->bugs & UART_BUG_TXEN) {
> -			unsigned char lsr, iir;
> +			unsigned char lsr;
>  			lsr = serial_in(up, UART_LSR);
>  			up->lsr_saved_flags |= lsr & LSR_SAVE_FLAGS;
> -			iir = serial_in(up, UART_IIR) & 0x0f;
>  			if ((up->port.type == PORT_RM9000) ?
> -				(lsr & UART_LSR_THRE &&
> -				(iir == UART_IIR_NO_INT || iir == UART_IIR_THRI)) :
> -				(lsr & UART_LSR_TEMT && iir & UART_IIR_NO_INT))
> +				(lsr & UART_LSR_THRE) :
> +				(lsr & UART_LSR_TEMT))
>  				transmit_chars(up);
>  		}
>  	}

Does anyone have any comments about this please?

In a nutshell -- this is needed so that we make the UART_BUG_TXEN really 
harmless (which I guess it originally inteded to be, but reading IIR has 
some unwanted sideeffects and is in fact not needed).

We need to have this in to handle properly the cases in which BUG_TXEN is 
misdetected, and we can't blacklist such systems as we do for some SoL 
hardware (see commit b6adea334c6c (" 8250: fix boot hang with serial 
console when using with Serial Over Lan port"). Also there doesn't seem to 
be any straightforward way to workaround the misdetection, so this seems 
to be proper fix, unbreaking all the possible scenarios.

Alan, Greg, any comments?

Thanks,

-- 
Jiri Kosina
SUSE Labs, Novell Inc.