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

[Moteen Shah]

Hey Ilpo,

On 17/09/26 15:10, Ilpo Järvinen wrote:
> 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@xxxxxx>
> > > > ---
> > > >   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@xxxxxxxxxxxxx/#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?


The IP that TDA54 uses does not supports the shadow registers, hence we 
will have to go through the quirk/capability path.

Regards,
Moteen

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