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

[Ilpo Järvinen]

On Fri, 18 Sep 2026, Moteen Shah wrote:

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

Fair. Thanks for confirming.

-- 
 i.