On Wed, Jun 19, 2019 at 1:20 PM Andy Shevchenko
<andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
>
> On Wed, Jun 19, 2019 at 10:16:39AM +0200, Oliver Barta wrote:
> > This reverts commit 2e9fe539108320820016f78ca7704a7342788380.
> >
> > Reading LSR unconditionally but processing the error flags only if
> > UART_IIR_RDI bit was set before in IIR may lead to a loss of transmission
> > error information on UARTs where the transmission error flags are cleared
> > by a read of LSR. Information are lost in case an error is detected right
> > before the read of LSR while processing e.g. an UART_IIR_THRI interrupt.
> >
>
> Perhaps Fixes tag?
>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
>
Thank you for the review. I also thought about the Fixes tag but
finally decided not to use it. It is a simple revert, i.e. the subject
of the commit which would be mentioned by the Fixes tag is in the new
subject anyway and the commit ID is referred in the first line of the
commit message body. The Fixes tag would not add any additional
information. I also checked a couple of recent revert commits in the
kernel and noticed that many of them actually don't have this tag.
> > Signed-off-by: Oliver Barta <o.barta89@xxxxxxxxx>
> > ---
> > drivers/tty/serial/8250/8250_port.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> > index d2f3310abe54..682300713be4 100644
> > --- a/drivers/tty/serial/8250/8250_port.c
> > +++ b/drivers/tty/serial/8250/8250_port.c
> > @@ -1869,8 +1869,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
> >
> > status = serial_port_in(port, UART_LSR);
> >
> > - if (status & (UART_LSR_DR | UART_LSR_BI) &&
> > - iir & UART_IIR_RDI) {
> > + if (status & (UART_LSR_DR | UART_LSR_BI)) {
> > if (!up->dma || handle_rx_dma(up, iir))
> > status = serial8250_rx_chars(up, status);
> > }
> > --
> > 2.20.1
> >
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
Best regards,
Oliver Barta
