| Message ID | 20201210055257.1053028-1-alexander.sverdlin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | serial: 8250_omap: Avoid FIFO corruption caused by MDR1 access | expand |
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@vger.kernel.org > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com> Thanks for the fix. Reviewed-by: Vignesh Raghavendra <vigneshr@ti.com> > --- > 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); >
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);
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@vger.kernel.org Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com> --- drivers/tty/serial/8250/8250_omap.c | 5 ----- 1 file changed, 5 deletions(-)