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

From: Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
To: Phil Elwell <phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>,
	Greg Kroah-Hartman
	<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org>,
	Peter Hurley
	<peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org>,
	Yegor Yefremov
	<yegorslists-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>,
	Jan Kiszka <jan.kiszka-kv7WeFo6aLtBDgjK7y7TUQ@public.gmane.org>,
	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH] serial: 8250: Fix THRE flag usage for CAP_MINI
Date: Tue, 27 Jun 2017 12:18:33 +0300	[thread overview]
Message-ID: <1498555113.22624.182.camel@linux.intel.com> (raw)
In-Reply-To: <1498485581-72097-1-git-send-email-phil-FnsA7b+Nu9XbIbC87yuRow@public.gmane.org>

On Mon, 2017-06-26 at 14:59 +0100, Phil Elwell wrote:
> 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.
> 

>  		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;

Might this end up with two sequential read of LSR?

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

_______________________________________________
linux-rpi-kernel mailing list
linux-rpi-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rpi-kernel