mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Kumar, Udit" <u-kumar1@ti.com>
To: Yicong Yang <yang.yicong@picoheart.com>,
	<ilpo.jarvinen@linux.intel.com>,
	<andriy.shevchenko@linux.intel.com>,
	<linux-serial@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Cc: <gregkh@linuxfoundation.org>, <jirislaby@kernel.org>,
	<geshijian@picoheart.com>, <yangyang.8776@picoheart.com>,
	<yanligen@picoheart.com>, <u-kumar1@ti.com>
Subject: Re: [PATCH v3] serial: 8250_dw: Prefer SRBR in bogus RX timeout workaround if available
Date: Thu, 17 Sep 2026 16:17:02 +0530	[thread overview]
Message-ID: <52822f65-58c4-4def-a1b6-e37055d4c0c6@ti.com> (raw)
In-Reply-To: <20260908141524.20302-1-yang.yicong@picoheart.com>

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.

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

Thanks
Udit


> Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>
> ---
> Change since v1:
> - refine the comments and type per Andy
> Link: https://lore.kernel.org/linux-serial/20260907074035.77796-1-yang.yicong@picoheart.com/
> 
> 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@picoheart.com/
> 
>  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);


  parent reply	other threads:[~2026-09-17 10:47 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-08 14:15 Yicong Yang
2026-09-08 15:58 ` Andy Shevchenko
2026-09-17 10:47 ` Kumar, Udit [this message]
2026-09-17 13:06   ` Yicong Yang
2026-09-17 13:33     ` Kumar, Udit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=52822f65-58c4-4def-a1b6-e37055d4c0c6@ti.com \
    --to=u-kumar1@ti.com \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=geshijian@picoheart.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=jirislaby@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-serial@vger.kernel.org \
    --cc=yang.yicong@picoheart.com \
    --cc=yangyang.8776@picoheart.com \
    --cc=yanligen@picoheart.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®