All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-aspeed@lists.ozlabs.org
Subject: [PATCH] tty: 8250: Add UART_BUG_TXRACE workaround for Aspeed VUART
Date: Tue, 18 May 2021 08:38:06 +0200	[thread overview]
Message-ID: <YKNgziMXms4zWRoY@kroah.com> (raw)
In-Reply-To: <d7918dcf-b938-498c-a012-3d93a748431b@www.fastmail.com>

On Tue, May 18, 2021 at 11:00:39AM +0930, Andrew Jeffery wrote:
> 
> 
> On Mon, 17 May 2021, at 23:45, Greg KH wrote:
> > On Mon, May 17, 2021 at 10:11:05PM +0930, Andrew Jeffery wrote:
> > > Aspeed Virtual UARTs directly bridge e.g. the system console UART on the
> > > LPC bus to the UART interface on the BMC's internal APB. As such there's
> > > no RS-232 signalling involved - the UART interfaces on each bus are
> > > directly connected as the producers and consumers of the one set of
> > > FIFOs.
> > > 
> > > The APB in the AST2600 generally runs at 100MHz while the LPC bus peaks
> > > at 33MHz. The difference in clock speeds exposes a race in the VUART
> > > design where a Tx data burst on the APB interface can result in a byte
> > > lost on the LPC interface. The symptom is LSR[DR] remains clear on the
> > > LPC interface despite data being present in its Rx FIFO, while LSR[THRE]
> > > remains clear on the APB interface as the host has not consumed the data
> > > the BMC has transmitted. In this state, the UART has stalled and no
> > > further data can be transmitted without manual intervention (e.g.
> > > resetting the FIFOs, resulting in loss of data).
> > > 
> > > The recommended work-around is to insert a read cycle on the APB
> > > interface between writes to THR.
> > > 
> > > Cc: ChiaWei Wang <chiawei_wang@aspeedtech.com>
> > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > > ---
> > >  drivers/tty/serial/8250/8250.h              | 1 +
> > >  drivers/tty/serial/8250/8250_aspeed_vuart.c | 1 +
> > >  drivers/tty/serial/8250/8250_port.c         | 2 ++
> > >  3 files changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> > > index 52bb21205bb6..4d6f5e0ecd4c 100644
> > > --- a/drivers/tty/serial/8250/8250.h
> > > +++ b/drivers/tty/serial/8250/8250.h
> > > @@ -88,6 +88,7 @@ struct serial8250_config {
> > >  #define UART_BUG_NOMSR	(1 << 2)	/* UART has buggy MSR status bits (Au1x00) */
> > >  #define UART_BUG_THRE	(1 << 3)	/* UART has buggy THRE reassertion */
> > >  #define UART_BUG_PARITY	(1 << 4)	/* UART mishandles parity if FIFO enabled */
> > > +#define UART_BUG_TXRACE (1 << 5)	/* UART Tx fails to set remote DR */
> > 
> > BUG()?
> 
> Can you please expand on what you mean here? I don't follow.
> 
> At least, I think there might be a formatting issue (spaces vs tabs).

Ick, my fault, I meant "BIT()"?  To perhaps use that macro instead of the <<
symbol.

And yes, tabs would be good as well :)

thanks,

greg k-h



WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Andrew Jeffery <andrew@aj.id.au>
Cc: linux-serial@vger.kernel.org, Jiri Slaby <jirislaby@kernel.org>,
	Joel Stanley <joel@jms.id.au>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-aspeed@lists.ozlabs.org, openbmc@lists.ozlabs.org,
	jenmin_yuan@aspeedtech.com, Ryan Chen <ryan_chen@aspeedtech.com>,
	Milton Miller II <miltonm@us.ibm.com>,
	"Chia-Wei, Wang" <chiawei_wang@aspeedtech.com>
Subject: Re: [PATCH] tty: 8250: Add UART_BUG_TXRACE workaround for Aspeed VUART
Date: Tue, 18 May 2021 08:38:06 +0200	[thread overview]
Message-ID: <YKNgziMXms4zWRoY@kroah.com> (raw)
In-Reply-To: <d7918dcf-b938-498c-a012-3d93a748431b@www.fastmail.com>

On Tue, May 18, 2021 at 11:00:39AM +0930, Andrew Jeffery wrote:
> 
> 
> On Mon, 17 May 2021, at 23:45, Greg KH wrote:
> > On Mon, May 17, 2021 at 10:11:05PM +0930, Andrew Jeffery wrote:
> > > Aspeed Virtual UARTs directly bridge e.g. the system console UART on the
> > > LPC bus to the UART interface on the BMC's internal APB. As such there's
> > > no RS-232 signalling involved - the UART interfaces on each bus are
> > > directly connected as the producers and consumers of the one set of
> > > FIFOs.
> > > 
> > > The APB in the AST2600 generally runs at 100MHz while the LPC bus peaks
> > > at 33MHz. The difference in clock speeds exposes a race in the VUART
> > > design where a Tx data burst on the APB interface can result in a byte
> > > lost on the LPC interface. The symptom is LSR[DR] remains clear on the
> > > LPC interface despite data being present in its Rx FIFO, while LSR[THRE]
> > > remains clear on the APB interface as the host has not consumed the data
> > > the BMC has transmitted. In this state, the UART has stalled and no
> > > further data can be transmitted without manual intervention (e.g.
> > > resetting the FIFOs, resulting in loss of data).
> > > 
> > > The recommended work-around is to insert a read cycle on the APB
> > > interface between writes to THR.
> > > 
> > > Cc: ChiaWei Wang <chiawei_wang@aspeedtech.com>
> > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > > ---
> > >  drivers/tty/serial/8250/8250.h              | 1 +
> > >  drivers/tty/serial/8250/8250_aspeed_vuart.c | 1 +
> > >  drivers/tty/serial/8250/8250_port.c         | 2 ++
> > >  3 files changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> > > index 52bb21205bb6..4d6f5e0ecd4c 100644
> > > --- a/drivers/tty/serial/8250/8250.h
> > > +++ b/drivers/tty/serial/8250/8250.h
> > > @@ -88,6 +88,7 @@ struct serial8250_config {
> > >  #define UART_BUG_NOMSR	(1 << 2)	/* UART has buggy MSR status bits (Au1x00) */
> > >  #define UART_BUG_THRE	(1 << 3)	/* UART has buggy THRE reassertion */
> > >  #define UART_BUG_PARITY	(1 << 4)	/* UART mishandles parity if FIFO enabled */
> > > +#define UART_BUG_TXRACE (1 << 5)	/* UART Tx fails to set remote DR */
> > 
> > BUG()?
> 
> Can you please expand on what you mean here? I don't follow.
> 
> At least, I think there might be a formatting issue (spaces vs tabs).

Ick, my fault, I meant "BIT()"?  To perhaps use that macro instead of the <<
symbol.

And yes, tabs would be good as well :)

thanks,

greg k-h



WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Andrew Jeffery <andrew@aj.id.au>
Cc: Ryan Chen <ryan_chen@aspeedtech.com>,
	linux-aspeed@lists.ozlabs.org, openbmc@lists.ozlabs.org,
	linux-kernel@vger.kernel.org, linux-serial@vger.kernel.org,
	jenmin_yuan@aspeedtech.com, Jiri Slaby <jirislaby@kernel.org>,
	"Chia-Wei, Wang" <chiawei_wang@aspeedtech.com>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] tty: 8250: Add UART_BUG_TXRACE workaround for Aspeed VUART
Date: Tue, 18 May 2021 08:38:06 +0200	[thread overview]
Message-ID: <YKNgziMXms4zWRoY@kroah.com> (raw)
In-Reply-To: <d7918dcf-b938-498c-a012-3d93a748431b@www.fastmail.com>

On Tue, May 18, 2021 at 11:00:39AM +0930, Andrew Jeffery wrote:
> 
> 
> On Mon, 17 May 2021, at 23:45, Greg KH wrote:
> > On Mon, May 17, 2021 at 10:11:05PM +0930, Andrew Jeffery wrote:
> > > Aspeed Virtual UARTs directly bridge e.g. the system console UART on the
> > > LPC bus to the UART interface on the BMC's internal APB. As such there's
> > > no RS-232 signalling involved - the UART interfaces on each bus are
> > > directly connected as the producers and consumers of the one set of
> > > FIFOs.
> > > 
> > > The APB in the AST2600 generally runs at 100MHz while the LPC bus peaks
> > > at 33MHz. The difference in clock speeds exposes a race in the VUART
> > > design where a Tx data burst on the APB interface can result in a byte
> > > lost on the LPC interface. The symptom is LSR[DR] remains clear on the
> > > LPC interface despite data being present in its Rx FIFO, while LSR[THRE]
> > > remains clear on the APB interface as the host has not consumed the data
> > > the BMC has transmitted. In this state, the UART has stalled and no
> > > further data can be transmitted without manual intervention (e.g.
> > > resetting the FIFOs, resulting in loss of data).
> > > 
> > > The recommended work-around is to insert a read cycle on the APB
> > > interface between writes to THR.
> > > 
> > > Cc: ChiaWei Wang <chiawei_wang@aspeedtech.com>
> > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > > ---
> > >  drivers/tty/serial/8250/8250.h              | 1 +
> > >  drivers/tty/serial/8250/8250_aspeed_vuart.c | 1 +
> > >  drivers/tty/serial/8250/8250_port.c         | 2 ++
> > >  3 files changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> > > index 52bb21205bb6..4d6f5e0ecd4c 100644
> > > --- a/drivers/tty/serial/8250/8250.h
> > > +++ b/drivers/tty/serial/8250/8250.h
> > > @@ -88,6 +88,7 @@ struct serial8250_config {
> > >  #define UART_BUG_NOMSR	(1 << 2)	/* UART has buggy MSR status bits (Au1x00) */
> > >  #define UART_BUG_THRE	(1 << 3)	/* UART has buggy THRE reassertion */
> > >  #define UART_BUG_PARITY	(1 << 4)	/* UART mishandles parity if FIFO enabled */
> > > +#define UART_BUG_TXRACE (1 << 5)	/* UART Tx fails to set remote DR */
> > 
> > BUG()?
> 
> Can you please expand on what you mean here? I don't follow.
> 
> At least, I think there might be a formatting issue (spaces vs tabs).

Ick, my fault, I meant "BIT()"?  To perhaps use that macro instead of the <<
symbol.

And yes, tabs would be good as well :)

thanks,

greg k-h



WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Andrew Jeffery <andrew@aj.id.au>
Cc: linux-serial@vger.kernel.org, Jiri Slaby <jirislaby@kernel.org>,
	Joel Stanley <joel@jms.id.au>,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-aspeed@lists.ozlabs.org, openbmc@lists.ozlabs.org,
	jenmin_yuan@aspeedtech.com, Ryan Chen <ryan_chen@aspeedtech.com>,
	Milton Miller II <miltonm@us.ibm.com>,
	"Chia-Wei, Wang" <chiawei_wang@aspeedtech.com>
Subject: Re: [PATCH] tty: 8250: Add UART_BUG_TXRACE workaround for Aspeed VUART
Date: Tue, 18 May 2021 08:38:06 +0200	[thread overview]
Message-ID: <YKNgziMXms4zWRoY@kroah.com> (raw)
In-Reply-To: <d7918dcf-b938-498c-a012-3d93a748431b@www.fastmail.com>

On Tue, May 18, 2021 at 11:00:39AM +0930, Andrew Jeffery wrote:
> 
> 
> On Mon, 17 May 2021, at 23:45, Greg KH wrote:
> > On Mon, May 17, 2021 at 10:11:05PM +0930, Andrew Jeffery wrote:
> > > Aspeed Virtual UARTs directly bridge e.g. the system console UART on the
> > > LPC bus to the UART interface on the BMC's internal APB. As such there's
> > > no RS-232 signalling involved - the UART interfaces on each bus are
> > > directly connected as the producers and consumers of the one set of
> > > FIFOs.
> > > 
> > > The APB in the AST2600 generally runs at 100MHz while the LPC bus peaks
> > > at 33MHz. The difference in clock speeds exposes a race in the VUART
> > > design where a Tx data burst on the APB interface can result in a byte
> > > lost on the LPC interface. The symptom is LSR[DR] remains clear on the
> > > LPC interface despite data being present in its Rx FIFO, while LSR[THRE]
> > > remains clear on the APB interface as the host has not consumed the data
> > > the BMC has transmitted. In this state, the UART has stalled and no
> > > further data can be transmitted without manual intervention (e.g.
> > > resetting the FIFOs, resulting in loss of data).
> > > 
> > > The recommended work-around is to insert a read cycle on the APB
> > > interface between writes to THR.
> > > 
> > > Cc: ChiaWei Wang <chiawei_wang@aspeedtech.com>
> > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> > > ---
> > >  drivers/tty/serial/8250/8250.h              | 1 +
> > >  drivers/tty/serial/8250/8250_aspeed_vuart.c | 1 +
> > >  drivers/tty/serial/8250/8250_port.c         | 2 ++
> > >  3 files changed, 4 insertions(+)
> > > 
> > > diff --git a/drivers/tty/serial/8250/8250.h b/drivers/tty/serial/8250/8250.h
> > > index 52bb21205bb6..4d6f5e0ecd4c 100644
> > > --- a/drivers/tty/serial/8250/8250.h
> > > +++ b/drivers/tty/serial/8250/8250.h
> > > @@ -88,6 +88,7 @@ struct serial8250_config {
> > >  #define UART_BUG_NOMSR	(1 << 2)	/* UART has buggy MSR status bits (Au1x00) */
> > >  #define UART_BUG_THRE	(1 << 3)	/* UART has buggy THRE reassertion */
> > >  #define UART_BUG_PARITY	(1 << 4)	/* UART mishandles parity if FIFO enabled */
> > > +#define UART_BUG_TXRACE (1 << 5)	/* UART Tx fails to set remote DR */
> > 
> > BUG()?
> 
> Can you please expand on what you mean here? I don't follow.
> 
> At least, I think there might be a formatting issue (spaces vs tabs).

Ick, my fault, I meant "BIT()"?  To perhaps use that macro instead of the <<
symbol.

And yes, tabs would be good as well :)

thanks,

greg k-h



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2021-05-18  6:38 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-17 12:41 [PATCH] tty: 8250: Add UART_BUG_TXRACE workaround for Aspeed VUART Andrew Jeffery
2021-05-17 12:41 ` Andrew Jeffery
2021-05-17 12:41 ` Andrew Jeffery
2021-05-17 12:41 ` Andrew Jeffery
2021-05-17 14:15 ` Greg KH
2021-05-17 14:15   ` Greg KH
2021-05-17 14:15   ` Greg KH
2021-05-17 14:15   ` Greg KH
2021-05-18  1:30   ` Andrew Jeffery
2021-05-18  1:30     ` Andrew Jeffery
2021-05-18  1:30     ` Andrew Jeffery
2021-05-18  1:30     ` Andrew Jeffery
2021-05-18  6:38     ` Greg Kroah-Hartman [this message]
2021-05-18  6:38       ` Greg Kroah-Hartman
2021-05-18  6:38       ` Greg Kroah-Hartman
2021-05-18  6:38       ` Greg Kroah-Hartman
2021-05-18 23:39       ` Andrew Jeffery
2021-05-18 23:39         ` Andrew Jeffery
2021-05-18 23:39         ` Andrew Jeffery
2021-05-18 23:39         ` Andrew Jeffery

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=YKNgziMXms4zWRoY@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=linux-aspeed@lists.ozlabs.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.