Re: [PATCH v3] serial: 8250_dw: Prefer SRBR in bogus RX timeout workaround if available

[Kumar, Udit]

On 9/17/2026 6:36 PM, Yicong Yang wrote:
> On 9/17/26 6:47 PM, Kumar, Udit wrote:
>> Hi Yicong,
>>
>> On 9/8/2026 7:45 PM, Yicong Yang wrote:
>>> The DW uart could get into the cases where a bogus RX timeout
>>> interrupt is asserted but no available data. This could be
>>> workaround by doing a bogus read.
>>>
>>> Currently the driver's using the standard RBR (receive buffer
>>> register) for this bogus read. However the reading of RBR
>>> in this case is allowed to raise a hardware error if vendor
>>> choose to implement in this way (our platform). It's also
>>> allowed to do the bogus read using SRBR (shadow RBR) for
>>> workaround which won't raise the hardware error. So change
>>> to use the SRBR to workaround the issue if it's available.
>>>
>>
>> Thanks for the patch. If I understand correctly, the IP you are using
>> does not allow reading an empty FIFO, so you decided to read the SRBR
>> instead.
> 
> yes but under certain condition, see blow.
> 
>>
>> Does this same case applies when you open a port without any data in the
>> FIFO:
>> https://elixir.bootlin.com/linux/v7.3-rc3/source/drivers/tty/serial/8250/8250_port.c#L710
>> ?
>>
>> If yes then I am wondering, how the condition mentioned above is bypassed
> 
> reading an empty FIFO (by RBR) will be fine when the fifo is disabled.
> so I think serial8250_clear_interrupts() you referred is always invoked
> when the FIFO is disabled.
> 
> I also find a similar issue related to serial8250_clear_interrupts()
> that trigger the PSLVERR when reading the empty FIFO when the FIFO
> is enabled. this is fixed by 7f8fdd4dbffc ("serial: 8250: fix panic due to PSLVERR")
> 

Thanks for details, so now you are making sure, while at this empty read
fifo is disabled.


> thanks.
> 
>>
>> Thanks
>> Udit
>>
>>
>>> Signed-off-by: Yicong Yang <yang.yicong@xxxxxxxxxxxxx>
>>> ---
>>> Change since v1:
>>> - refine the comments and type per Andy
>>> Link: https://lore.kernel.org/linux-serial/20260907074035.77796-1-yang.yicong@xxxxxxxxxxxxx/
>>>
>>> Change since v1:
>>> - simply the workaround path per Ilpo and Andy
>>> - run ~48k reboot tests and didn't reproduce the issue
>>> Link: https://lore.kernel.org/linux-serial/20260629075510.32854-1-yang.yicong@xxxxxxxxxxxxx/
>>>
>>>  drivers/tty/serial/8250/8250_dw.c    | 9 ++++++++-
>>>  drivers/tty/serial/8250/8250_dwlib.c | 5 +++++
>>>  drivers/tty/serial/8250/8250_dwlib.h | 5 +++++
>>>  3 files changed, 18 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
>>> index 51d026f20825..b8a0bca8b536 100644
>>> --- a/drivers/tty/serial/8250/8250_dw.c
>>> +++ b/drivers/tty/serial/8250/8250_dw.c
>>> @@ -439,8 +439,15 @@ static int dw8250_handle_irq(struct uart_port *p)
>>>  	if (!up->dma && rx_timeout) {
>>>  		status = serial_lsr_in(up);
>>>  
>>> +		/*
>>> +		 * Do the bogus read from Shadow RBR (SRBR) if provided.
>>> +		 * Both RBR and SRBR are supported ways to workaround
>>> +		 * this problem. But read RBR can cause hardware error
>>> +		 * (PSLVERR) if hardware choose to implement in this way
>>> +		 * (implement REG_TIMEOUT_WIDTH to 0), whereas SRBR won't.
>>> +		 */
>>>  		if (!(status & (UART_LSR_DR | UART_LSR_BI)))
>>> -			serial_port_in(p, UART_RX);
>>> +			serial_port_in(p, d->data.srbr);
>>>  	}
>>>  
>>>  	/* Manually stop the Rx DMA transfer when acting as flow controller */
>>> diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c
>>> index 8859e66d2d71..9bb02a4ab11f 100644
>>> --- a/drivers/tty/serial/8250/8250_dwlib.c
>>> +++ b/drivers/tty/serial/8250/8250_dwlib.c
>>> @@ -247,5 +247,10 @@ void dw8250_setup_port(struct uart_port *p)
>>>  
>>>  	if (reg & DW_UART_CPR_SIR_MODE)
>>>  		up->capabilities |= UART_CAP_IRDA;
>>> +
>>> +	if (reg & DW_UART_CPR_SHADOW)
>>> +		pd->srbr = DW_UART_SRBR_0;
>>> +	else
>>> +		pd->srbr = UART_RX;
>>>  }
>>>  EXPORT_SYMBOL_GPL(dw8250_setup_port);
>>> diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h
>>> index 1fe52332e774..ee7a07fac0f6 100644
>>> --- a/drivers/tty/serial/8250/8250_dwlib.h
>>> +++ b/drivers/tty/serial/8250/8250_dwlib.h
>>> @@ -13,6 +13,7 @@
>>>  #include "8250.h"
>>>  
>>>  /* Offsets for the DesignWare specific registers */
>>> +#define DW_UART_SRBR_0	0x0c /* Shadow Receive Buffer Register */
>>>  #define DW_UART_USR	0x1f /* UART Status Register */
>>>  #define DW_UART_DMASA	0xa8 /* DMA Software Ack */
>>>  #define DW_UART_TCR	0xac /* Transceiver Control Register (RS485) */
>>> @@ -90,6 +91,10 @@ struct dw8250_port_data {
>>>  
>>>  	/* RS485 variables */
>>>  	bool			hw_rs485_support;
>>> +
>>> +	/* Register offsets */
>>> +	/* Shadow RBR (if not defined RBR will be used) */
>>> +	int		srbr;
>>>  };
>>>  
>>>  void dw8250_do_set_termios(struct uart_port *p, struct ktermios *termios, const struct ktermios *old);