linux-kernel - Re: [PATCH tty-next v3 1/6] serial: 8250: Adjust the timeout for FIFO mode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.21.2410310349450.40463@angie.orcam.me.uk>
Date: Thu, 31 Oct 2024 04:44:46 +0000 (GMT)
From: "Maciej W. Rozycki" <macro@...am.me.uk>
To: Jiri Slaby <jirislaby@...nel.org>
cc: John Ogness <john.ogness@...utronix.de>, 
    Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
    Petr Mladek <pmladek@...e.com>, 
    Sergey Senozhatsky <senozhatsky@...omium.org>, 
    Steven Rostedt <rostedt@...dmis.org>, Thomas Gleixner <tglx@...utronix.de>, 
    Esben Haabendal <esben@...nix.com>, linux-serial@...r.kernel.org, 
    linux-kernel@...r.kernel.org, 
    Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, 
    Rengarajan S <rengarajan.s@...rochip.com>, 
    Jeff Johnson <quic_jjohnson@...cinc.com>, 
    Serge Semin <fancer.lancer@...il.com>, 
    Lino Sanfilippo <l.sanfilippo@...bus.com>, 
    Wander Lairson Costa <wander@...hat.com>
Subject: Re: [PATCH tty-next v3 1/6] serial: 8250: Adjust the timeout for
 FIFO mode

On Wed, 30 Oct 2024, Jiri Slaby wrote:

> > @@ -3306,13 +3310,18 @@ static void serial8250_console_restore(struct
> > uart_8250_port *up)
> >   static void serial8250_console_fifo_write(struct uart_8250_port *up,
> >   					  const char *s, unsigned int count)
> >   {
> > -	int i;
> >   	const char *end = s + count;
> >   	unsigned int fifosize = up->tx_loadsz;
> > +	unsigned int tx_count = 0;
> >   	bool cr_sent = false;
> > +	unsigned int i;
> >     	while (s != end) {
> > -		wait_for_lsr(up, UART_LSR_THRE);
> > +		/* Allow timeout for each byte of a possibly full FIFO. */
> > +		for (i = 0; i < fifosize; i++) {
> > +			if (wait_for_lsr(up, UART_LSR_THRE))
> > +				break;
> > +		}
> 
> THRE only signals there is a space for one character.

 Nope[1]:

"In the FIFO mode, THRE is set when the transmit FIFO is empty; it is 
cleared when at least one byte is written to the transmit FIFO."

It seems common enough a misconception that once I actually had to fix the 
bad interpretation of THRE in an unpublished platform driver to get decent 
performance out of it at higher rates such as 230400bps, as it only pushed 
one byte at a time to the FIFO while it had it all available once THRE has 
been set.

> > +	/* Allow timeout for each byte written. */
> > +	for (i = 0; i < tx_count; i++) {
> > +		if (wait_for_lsr(up, UART_LSR_THRE))
> 
> This ensures you sent one character from the FIFO. The FIFO still can contain
> plenty of them. Did you want UART_LSR_TEMT?

 The difference between THRE and TEMT is the state of the shift register 
only[2]:

"In the FIFO mode, TEMT is set when the transmitter FIFO and shift 
register are both empty."

References:

[1] "TL16C550C, TL16C550CI Asynchronous Communications Element with 
    Autoflow Control", Texas Instruments, SLLS177F -- March 1994 -- 
    Revised March 2001, p. 30

[2] same

  Maciej