diff mbox

[1/8] tty: serial: fsl_lpuart: consider TX FIFO too in tx_empty

Message ID 20160609151039.20817-2-bhuvanchandra.dv@toradex.com (mailing list archive)
State New, archived
Headers show

Commit Message

Bhuvanchandra DV June 9, 2016, 3:10 p.m. UTC
From: Stefan Agner <stefan@agner.ch>

Currently the tx_empty callback only considers the Transmit Complete
Flag (TC). The reference manual is not quite clear if the TC flag
covers the TX FIFO too. Debug prints on real hardware have shown that
from time to time the TC flag is asserted (indicating Transmitter
idle) while there are still data in the TX FIFO. Hence, in this case
the serial core will call the shutdown callback even though there are
data remaining in the TX FIFO buffers.

Avoid early shutdowns by considering the TX FIFO empty flag too. Also
avoid theoretical race conditions between DMA and the driver by
checking whether the TX DMA is in progress too.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/tty/serial/fsl_lpuart.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Comments

Greg Kroah-Hartman June 25, 2016, 9:26 p.m. UTC | #1
On Thu, Jun 09, 2016 at 08:40:32PM +0530, Bhuvanchandra DV wrote:
> From: Stefan Agner <stefan@agner.ch>
> 
> Currently the tx_empty callback only considers the Transmit Complete
> Flag (TC). The reference manual is not quite clear if the TC flag
> covers the TX FIFO too. Debug prints on real hardware have shown that
> from time to time the TC flag is asserted (indicating Transmitter
> idle) while there are still data in the TX FIFO. Hence, in this case
> the serial core will call the shutdown callback even though there are
> data remaining in the TX FIFO buffers.
> 
> Avoid early shutdowns by considering the TX FIFO empty flag too. Also
> avoid theoretical race conditions between DMA and the driver by
> checking whether the TX DMA is in progress too.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/tty/serial/fsl_lpuart.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)

Why are you not signing off on patches that are flowing through you?
Please fix this up and resend the series, after breaking up the clock
change as asked.

thanks,

greg k-h
Bhuvanchandra DV June 28, 2016, 3:54 a.m. UTC | #2
On 06/26/16 02:56, Greg KH wrote:

> On Thu, Jun 09, 2016 at 08:40:32PM +0530, Bhuvanchandra DV wrote:
>> From: Stefan Agner <stefan@agner.ch>
>>
>> Currently the tx_empty callback only considers the Transmit Complete
>> Flag (TC). The reference manual is not quite clear if the TC flag
>> covers the TX FIFO too. Debug prints on real hardware have shown that
>> from time to time the TC flag is asserted (indicating Transmitter
>> idle) while there are still data in the TX FIFO. Hence, in this case
>> the serial core will call the shutdown callback even though there are
>> data remaining in the TX FIFO buffers.
>>
>> Avoid early shutdowns by considering the TX FIFO empty flag too. Also
>> avoid theoretical race conditions between DMA and the driver by
>> checking whether the TX DMA is in progress too.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>   drivers/tty/serial/fsl_lpuart.c | 14 ++++++++++++--
>>   1 file changed, 12 insertions(+), 2 deletions(-)
> Why are you not signing off on patches that are flowing through you?
> Please fix this up and resend the series, after breaking up the clock
> change as asked.

Okay

>
> thanks,
>
> greg k-h
diff mbox

Patch

diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 3d79003..fabfa7e 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -810,8 +810,18 @@  static irqreturn_t lpuart32_int(int irq, void *dev_id)
 /* return TIOCSER_TEMT when transmitter is not busy */
 static unsigned int lpuart_tx_empty(struct uart_port *port)
 {
-	return (readb(port->membase + UARTSR1) & UARTSR1_TC) ?
-		TIOCSER_TEMT : 0;
+	struct lpuart_port *sport = container_of(port,
+			struct lpuart_port, port);
+	unsigned char sr1 = readb(port->membase + UARTSR1);
+	unsigned char sfifo = readb(port->membase + UARTSFIFO);
+
+	if (sport->dma_tx_in_progress)
+		return 0;
+
+	if (sr1 & UARTSR1_TC && sfifo & UARTSFIFO_TXEMPT)
+		return TIOCSER_TEMT;
+
+	return 0;
 }
 
 static unsigned int lpuart32_tx_empty(struct uart_port *port)