Re: [PATCH v2 2/2] serial: 8250_dw: Add capability to skip empty FIFO read

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2437 bytes --]

On Wed, 16 Sep 2026, Kumar, Udit wrote:
> On 9/16/2026 4:22 PM, Ilpo Järvinen wrote:
> > On Wed, 16 Sep 2026, Moteen Shah wrote:
> > 
> >> dw8250_handle_irq() does a bogus RX read on RX_TIMEOUT with no data
> >> present, to avoid an interrupt storm. The UART core also performs
> >> unconditional reads on the empty FIFO during startup and shutdown
> >> of the port. On the IP version used in TDA54, that interrupt storm
> >> no longer occurs, but reading an empty FIFO instead triggers a data
> >> abort.
> >>
> >> Add a new capability to guard against the empty FIFO reads, avoiding
> >> the data aborts.
> >>
> >> Signed-off-by: Moteen Shah <m-shah@ti.com>
> >> ---
> >>  drivers/tty/serial/8250/8250.h      |  1 +
> >>  drivers/tty/serial/8250/8250_dw.c   | 16 +++++++++++++++-
> >>  drivers/tty/serial/8250/8250_port.c | 12 +++++++++---
> >>  3 files changed, 25 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> >> index 77fe0588fd6b..45e13c3a8c14 100644
> >> --- a/drivers/tty/serial/8250/8250.h
> >> +++ b/drivers/tty/serial/8250/8250.h
> >> @@ -86,6 +86,7 @@ struct serial8250_config {
> >>  					 * STOP PARITY EPAR SPAR WLEN5 WLEN6
> >>  					 */
> >>  #define UART_CAP_NOTEMT	BIT(18)	/* UART without interrupt on TEMT available */
> >> +#define UART_CAP_RXFIFO_EMPTY_READ	BIT(19) /* UART needs LSR_DR check before RX read (TDA54) */
> > 
> > IMO, this define naming contradicts with the comment because you 
> > effectively say "capable of reading Rx while receive buffer is empty", not 
> > that it needs DR check before issuing that read on buffer (~ named exactly 
> > opposite of the actual meaning it is being used in the code).
> > 
> 
> I echo your comments; the naming should reflect that the IP does not
> allow reading an empty FIFO.

I now recalled we've also the pending RBR -> SRBR change:

https://lore.kernel.org/linux-serial/20260908141524.20302-1-yang.yicong@picoheart.com/#t

I wonder if a similar thing would help in this case as well?

I understand this patch covers more ground than the SRBR patch but can the 
IP used in this case read SRBR without aborting?

> At the same time, I am thinking, reading the FIFO based on the
> UART_LSR_DR bit being set in the serial8250_clear_interrupts and
> serial8250_do_shutdown functions.
> Do you see any side effects to this?



-- 
 i.