Re: [PATCH] serial: 8250_omap: Avoid FIFO corruption caused by MDR1 access

[Vignesh Raghavendra]

Hi,

On 12/10/20 11:22 AM, Alexander Sverdlin wrote:
> It has been observed that once per 300-1300 port openings the first
> transmitted byte is being corrupted on AM3352 ("v" written to FIFO appeared
> as "e" on the wire). It only happened if single byte has been transmitted
> right after port open, which means, DMA is not used for this transfer and
> the corruption never happened afterwards.
> 
> Therefore I've carefully re-read the MDR1 errata (link below), which says
> "when accessing the MDR1 registers that causes a dummy under-run condition
> that will freeze the UART in IrDA transmission. In UART mode, this may
> corrupt the transferred data". Strictly speaking,
> omap_8250_mdr1_errataset() performs a read access and if the value is the
> same as should be written, exits without errata-recommended FIFO reset.
> 
> A brief check of the serial_omap_mdr1_errataset() from the competing
> omap-serial driver showed it has no read access of MDR1. After removing the
> read access from omap_8250_mdr1_errataset() the data corruption never
> happened any more.
> 
> Link: https://www.ti.com/lit/er/sprz360i/sprz360i.pdf
> Fixes: 61929cf0169d ("tty: serial: Add 8250-core based omap driver")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@xxxxxxxxx>

Thanks for the fix.

Reviewed-by: Vignesh Raghavendra <vigneshr@xxxxxx>

> ---
>  drivers/tty/serial/8250/8250_omap.c | 5 -----
>  1 file changed, 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
> index 562087df7d33..0cc6d35a0815 100644
> --- a/drivers/tty/serial/8250/8250_omap.c
> +++ b/drivers/tty/serial/8250/8250_omap.c
> @@ -184,11 +184,6 @@ static void omap_8250_mdr1_errataset(struct uart_8250_port *up,
>  				     struct omap8250_priv *priv)
>  {
>  	u8 timeout = 255;
> -	u8 old_mdr1;
> -
> -	old_mdr1 = serial_in(up, UART_OMAP_MDR1);
> -	if (old_mdr1 == priv->mdr1)
> -		return;
>  
>  	serial_out(up, UART_OMAP_MDR1, priv->mdr1);
>  	udelay(2);
>