[PATCH tty-next v3 2/6] serial: 8250: Use high-level write function for FIFO

John Ogness posted 6 patches 1 month ago
[PATCH tty-next v3 2/6] serial: 8250: Use high-level write function for FIFO
Posted by John Ogness 1 month ago
Currently serial8250_console_fifo_write() directly writes into the
UART_TX register, rather than using the high-level function
serial8250_console_putchar(). This is because
serial8250_console_putchar() waits for the holding register to
become empty. That would defeat the purpose of the FIFO code.

Move the LSR_THRE waiting to a new function
serial8250_console_wait_putchar() so that the FIFO code can use
serial8250_console_putchar(). This will be particularly important
for a follow-up commit, where output bytes are inspected to track
newlines.

This is only refactoring and has no functional change.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 drivers/tty/serial/8250/8250_port.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
index adc48eeeac2b..8f7c9968ad41 100644
--- a/drivers/tty/serial/8250/8250_port.c
+++ b/drivers/tty/serial/8250/8250_port.c
@@ -3268,11 +3268,16 @@ EXPORT_SYMBOL_GPL(serial8250_set_defaults);
 #ifdef CONFIG_SERIAL_8250_CONSOLE
 
 static void serial8250_console_putchar(struct uart_port *port, unsigned char ch)
+{
+	serial_port_out(port, UART_TX, ch);
+}
+
+static void serial8250_console_wait_putchar(struct uart_port *port, unsigned char ch)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
 
 	wait_for_xmitr(up, UART_LSR_THRE);
-	serial_port_out(port, UART_TX, ch);
+	serial8250_console_putchar(port, ch);
 }
 
 /*
@@ -3312,6 +3317,7 @@ static void serial8250_console_fifo_write(struct uart_8250_port *up,
 {
 	const char *end = s + count;
 	unsigned int fifosize = up->tx_loadsz;
+	struct uart_port *port = &up->port;
 	unsigned int tx_count = 0;
 	bool cr_sent = false;
 	unsigned int i;
@@ -3325,10 +3331,10 @@ static void serial8250_console_fifo_write(struct uart_8250_port *up,
 
 		for (i = 0; i < fifosize && s != end; ++i) {
 			if (*s == '\n' && !cr_sent) {
-				serial_out(up, UART_TX, '\r');
+				serial8250_console_putchar(port, '\r');
 				cr_sent = true;
 			} else {
-				serial_out(up, UART_TX, *s++);
+				serial8250_console_putchar(port, *s++);
 				cr_sent = false;
 			}
 		}
@@ -3408,7 +3414,7 @@ void serial8250_console_write(struct uart_8250_port *up, const char *s,
 	if (likely(use_fifo))
 		serial8250_console_fifo_write(up, s, count);
 	else
-		uart_console_write(port, s, count, serial8250_console_putchar);
+		uart_console_write(port, s, count, serial8250_console_wait_putchar);
 
 	/*
 	 *	Finally, wait for transmitter to become empty
-- 
2.39.5
Re: [PATCH tty-next v3 2/6] serial: 8250: Use high-level write function for FIFO
Posted by Petr Mladek 2 weeks, 6 days ago
On Fri 2024-10-25 13:03:24, John Ogness wrote:
> Currently serial8250_console_fifo_write() directly writes into the
> UART_TX register, rather than using the high-level function
> serial8250_console_putchar(). This is because
> serial8250_console_putchar() waits for the holding register to
> become empty. That would defeat the purpose of the FIFO code.
> 
> Move the LSR_THRE waiting to a new function
> serial8250_console_wait_putchar() so that the FIFO code can use
> serial8250_console_putchar(). This will be particularly important
> for a follow-up commit, where output bytes are inspected to track
> newlines.
> 
> This is only refactoring and has no functional change.
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

Looks good:

Reviewed-by: Petr Mladek <pmladek@suse.com>

Best Regards,
Petr
Re: [PATCH tty-next v3 2/6] serial: 8250: Use high-level write function for FIFO
Posted by Andy Shevchenko 1 month ago
On Fri, Oct 25, 2024 at 01:03:24PM +0206, John Ogness wrote:
> Currently serial8250_console_fifo_write() directly writes into the
> UART_TX register, rather than using the high-level function
> serial8250_console_putchar(). This is because
> serial8250_console_putchar() waits for the holding register to
> become empty. That would defeat the purpose of the FIFO code.

You can slightly reformat the above to make it less shaky in terms
of the efficiency of a line capacity usage.

Currently serial8250_console_fifo_write() directly writes into
the UART_TX register, rather than using the high-level function
serial8250_console_putchar(). This is because
serial8250_console_putchar() waits for the holding register
to become empty. That would defeat the purpose of the FIFO code.

> Move the LSR_THRE waiting to a new function
> serial8250_console_wait_putchar() so that the FIFO code can use
> serial8250_console_putchar(). This will be particularly important
> for a follow-up commit, where output bytes are inspected to track
> newlines.

> This is only refactoring and has no functional change.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko