diff mbox series

[v2,1/1] serial: 8250_fsl: Fix TX interrupt handling condition

Message ID 20200928102841.43782-2-aabyzov@slb.com
State Superseded
Headers show
Series serial: 8250_fsl: Fix TX interrupt handling condition | expand

Commit Message

drolevar@gmail.com Sept. 28, 2020, 10:28 a.m. UTC 
From: Andrij Abyzov <aabyzov@slb.com>

This is the port of the commit db1b5bc047b3 ("serial: 8250: Fix TX
interrupt handling condition") to the 8250_fsl irq handling logic.

Interrupt handler checked THRE bit (transmitter holding register
empty) in LSR to detect if TX fifo is empty.
In case when there is only receive interrupts the TX handling
got called because THRE bit in LSR is set when there is no
transmission (FIFO empty). TX handling caused TX stop, which in
RS-485 half-duplex mode actually resets receiver FIFO. This is not
desired during reception because of possible data loss.

The fix is to check if THRI is set in IER in addition of the TX
fifo status. THRI in IER is set when TX is started and cleared
when TX is stopped.
This ensures that TX handling is only called when there is really
transmission on going and an interrupt for THRE and not when there
are only RX interrupts.

Signed-off-by: Andrij Abyzov <aabyzov@slb.com>
---
 drivers/tty/serial/8250/8250_fsl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Greg KH Sept. 28, 2020, 12:20 p.m. UTC | #1 
On Mon, Sep 28, 2020 at 12:28:41PM +0200, drolevar@gmail.com wrote:
> From: Andrij Abyzov <aabyzov@slb.com>

> 

> This is the port of the commit db1b5bc047b3 ("serial: 8250: Fix TX

> interrupt handling condition") to the 8250_fsl irq handling logic.

> 

> Interrupt handler checked THRE bit (transmitter holding register

> empty) in LSR to detect if TX fifo is empty.

> In case when there is only receive interrupts the TX handling

> got called because THRE bit in LSR is set when there is no

> transmission (FIFO empty). TX handling caused TX stop, which in

> RS-485 half-duplex mode actually resets receiver FIFO. This is not

> desired during reception because of possible data loss.

> 

> The fix is to check if THRI is set in IER in addition of the TX

> fifo status. THRI in IER is set when TX is started and cleared

> when TX is stopped.

> This ensures that TX handling is only called when there is really

> transmission on going and an interrupt for THRE and not when there

> are only RX interrupts.

> 

> Signed-off-by: Andrij Abyzov <aabyzov@slb.com>

> ---

>  drivers/tty/serial/8250/8250_fsl.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/drivers/tty/serial/8250/8250_fsl.c b/drivers/tty/serial/8250/8250_fsl.c

> index 0d0c80905c58..ceac6cfce4c7 100644

> --- a/drivers/tty/serial/8250/8250_fsl.c

> +++ b/drivers/tty/serial/8250/8250_fsl.c

> @@ -71,7 +71,7 @@ int fsl8250_handle_irq(struct uart_port *port)

>  

>  	serial8250_modem_status(up);

>  

> -	if (lsr & UART_LSR_THRE)

> +	if ((lsr & UART_LSR_THRE) && (up->ier & UART_IER_THRI))

>  		serial8250_tx_chars(up);

>  

>  	up->lsr_saved_flags = orig_lsr;

> -- 

> 2.25.1

> 


What changed from v1?

That goes below the --- line, as is asked for in the submitting patches
document, right?

Can you fix that up and send a v3?

Also, why are you forwarding on a patch that you did not write or sign
off on?  Please sign off on it yourself too.

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/tty/serial/8250/8250_fsl.c b/drivers/tty/serial/8250/8250_fsl.c
index 0d0c80905c58..ceac6cfce4c7 100644
--- a/drivers/tty/serial/8250/8250_fsl.c
+++ b/drivers/tty/serial/8250/8250_fsl.c
@@ -71,7 +71,7 @@  int fsl8250_handle_irq(struct uart_port *port)
 
 	serial8250_modem_status(up);
 
-	if (lsr & UART_LSR_THRE)
+	if ((lsr & UART_LSR_THRE) && (up->ier & UART_IER_THRI))
 		serial8250_tx_chars(up);
 
 	up->lsr_saved_flags = orig_lsr;