Re: [PATCH] serial: 8250: Fix THRE flag usage for CAP_MINI

[Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>]

Hi Phil,

Am 28.06.2017 um 10:42 schrieb Phil Elwell:
> The BCM2835 MINI UART has non-standard THRE semantics. Conventionally
> the bit means that the FIFO is empty (although there may still be a
> byte in the transmit register), but on 2835 it indicates that the FIFO
> is not full. This causes interrupts after every byte is transmitted,
> with the FIFO providing some interrupt latency tolerance.
>
> A consequence of this difference is that the usual strategy of writing
> multiple bytes into the TX FIFO after checking THRE once is unsafe.
> In the worst case of 7 bytes in the FIFO, writing 8 bytes loses all
> but the first since by then the FIFO is full.
>
> There is an HFIFO ("Hidden FIFO") capability that causes the transmit
> loop to terminate when both THRE and TEMT are set, i.e. when the TX
> block is completely idle. This is unnecessarily cautious, potentially
> causing gaps in transmission.
>
> Add a new conditional to the transmit loop, predicated on CAP_MINI,
> that exits when THRE is no longer set (the FIFO is full). This allows
> the FIFO to fill quickly but subsequent writes are paced by the
> transmission rate.
>
> Signed-off-by: Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>
> Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> Acked-by: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
>  drivers/tty/serial/8250/8250_port.c | 4 ++++
>  1 file changed, 4 insertions(+)

please increase the version of the patch and add your changelog below
this line, so Greg has the chance to apply the right fix.

Also i assume that Andy wanted to suggest you to add the fixes tag.

Sorry for the noise before.

Regards
Stefan

>
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index 4c620be..a5fe0e6 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1764,6 +1764,10 @@ void serial8250_tx_chars(struct uart_8250_port *up)
>  		if ((up->capabilities & UART_CAP_HFIFO) &&
>  		    (serial_in(up, UART_LSR) & BOTH_EMPTY) != BOTH_EMPTY)
>  			break;
> +		/* The BCM2835 MINI UART THRE bit is really a not-full bit. */
> +		if ((up->capabilities & UART_CAP_MINI) &&
> +		    !(serial_in(up, UART_LSR) & UART_LSR_THRE))
> +			break;
>  	} while (--count > 0);
>  
>  	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)