[PATCH v1 07/31] serial: 8250_mxupci: add GDL-based Rx routine for 8250_mxupci

Crescent Hsieh posted 31 patches 1 day, 11 hours ago
[PATCH v1 07/31] serial: 8250_mxupci: add GDL-based Rx routine for 8250_mxupci
Posted by Crescent Hsieh 1 day, 11 hours ago
Introduce a custom Rx routine using the GDL (Good Data Length) register to
read multiple bytes from the Rx FIFO on Moxa UPCI serial boards.

When no LSR error bits are present, the handler reads GDL bytes from
UART_RX and inserts them into the TTY flip buffer. If error bits are
detected, it falls back to the default serial8250_rx_chars() handler.

Signed-off-by: Crescent Hsieh <crescentcy.hsieh@moxa.com>
---
 drivers/tty/serial/8250/8250_mxupci.c | 37 ++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_mxupci.c b/drivers/tty/serial/8250/8250_mxupci.c
index 376c6d6135a3..ab6042f157c6 100644
--- a/drivers/tty/serial/8250/8250_mxupci.c
+++ b/drivers/tty/serial/8250/8250_mxupci.c
@@ -8,6 +8,7 @@
 
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/tty_flip.h>
 #include <linux/8250_pci.h>
 
 #include "8250.h"
@@ -42,6 +43,7 @@
 
 #define MOXA_UART_BASE_BAUD	921600
 #define MOXA_UART_OFFSET	8
+#define MOXA_UART_FIFO_SIZE	128
 
 /* Enhanced Function Register (EFR) */
 /*
@@ -70,6 +72,8 @@
 #define MOXA_UART_EFR_TX_FLOW_MASK	GENMASK(3, 2)
 #define MOXA_UART_EFR_PAGE_MASK		GENMASK(7, 6)
 
+#define MOXA_UART_GDL	0x07	/* Good data length register (enhanced mode only) */
+
 /* Enhanced Registers Page 0 */
 #define MOXA_UART_XON1	0x04
 #define MOXA_UART_XON2	0x05
@@ -212,6 +216,30 @@ static void mxupci8250_unthrottle(struct uart_port *port)
 	uart_port_unlock_irqrestore(port, flags);
 }
 
+static void mxupci8250_rx_chars(struct uart_8250_port *up)
+{
+	struct uart_port *port = &up->port;
+	struct tty_port *tport = &port->state->port;
+	int recv_room, gdl, i;
+	u8 buf[MOXA_UART_FIFO_SIZE];
+
+	recv_room = tty_buffer_request_room(tport, port->fifosize);
+
+	if (recv_room) {
+		gdl = serial_in(up, MOXA_UART_GDL);
+
+		if (gdl > recv_room)
+			gdl = recv_room;
+
+		for (i = 0; i < gdl; ++i)
+			buf[i] = serial_in(up, UART_RX);
+
+		port->icount.rx += gdl;
+		tty_insert_flip_string(tport, buf, gdl);
+		tty_flip_buffer_push(tport);
+	}
+}
+
 static int mxupci8250_handle_irq(struct uart_port *port)
 {
 	struct uart_8250_port *up = up_to_u8250p(port);
@@ -234,9 +262,12 @@ static int mxupci8250_handle_irq(struct uart_port *port)
 	    !(port->read_status_mask & UART_LSR_DR))
 		skip_rx = true;
 
-	if (lsr & (UART_LSR_DR | UART_LSR_BI) && !skip_rx)
-		lsr = serial8250_rx_chars(up, lsr);
-
+	if (lsr & (UART_LSR_DR | UART_LSR_BI) && !skip_rx) {
+		if (lsr & UART_LSR_BRK_ERROR_BITS)
+			lsr = serial8250_rx_chars(up, lsr);
+		else
+			mxupci8250_rx_chars(up);
+	}
 	serial8250_modem_status(up);
 
 	if ((lsr & UART_LSR_THRE) && (up->ier & UART_IER_THRI))
-- 
2.45.2
Re: [PATCH v1 07/31] serial: 8250_mxupci: add GDL-based Rx routine for 8250_mxupci
Posted by Andy Shevchenko 1 day, 4 hours ago
On Sun, Nov 30, 2025 at 12:43 PM Crescent Hsieh
<crescentcy.hsieh@moxa.com> wrote:
>
> Introduce a custom Rx routine using the GDL (Good Data Length) register to
> read multiple bytes from the Rx FIFO on Moxa UPCI serial boards.
>
> When no LSR error bits are present, the handler reads GDL bytes from
> UART_RX and inserts them into the TTY flip buffer. If error bits are
> detected, it falls back to the default serial8250_rx_chars() handler.

...

> +static void mxupci8250_rx_chars(struct uart_8250_port *up)
> +{
> +       struct uart_port *port = &up->port;
> +       struct tty_port *tport = &port->state->port;
> +       int recv_room, gdl, i;
> +       u8 buf[MOXA_UART_FIFO_SIZE];
> +
> +       recv_room = tty_buffer_request_room(tport, port->fifosize);

> +

Stray blank line addition.

> +       if (recv_room) {

Can't it be

  if (!recv_room)
    return;

?

> +               gdl = serial_in(up, MOXA_UART_GDL);

> +

Stray blank line addition.

> +               if (gdl > recv_room)
> +                       gdl = recv_room;
> +
> +               for (i = 0; i < gdl; ++i)
> +                       buf[i] = serial_in(up, UART_RX);
> +
> +               port->icount.rx += gdl;
> +               tty_insert_flip_string(tport, buf, gdl);
> +               tty_flip_buffer_push(tport);
> +       }
> +}

...

> -       if (lsr & (UART_LSR_DR | UART_LSR_BI) && !skip_rx)
> -               lsr = serial8250_rx_chars(up, lsr);
> -
> +       if (lsr & (UART_LSR_DR | UART_LSR_BI) && !skip_rx) {
> +               if (lsr & UART_LSR_BRK_ERROR_BITS)
> +                       lsr = serial8250_rx_chars(up, lsr);
> +               else
> +                       mxupci8250_rx_chars(up);
> +       }

Oh, can we reduce ping-pong a bit (the modification of the lines just
being added earlier in the same patch series)?

I think you can create a helper to wrap 8250_rx_chars() with split
version of the almost unreadable conditionals, this will also remove
the skip_rx variable

static ... _rx_chars()
{
  /* split version of skip_rx monstrous if */
  if ...
    return ...;
  if ...
   return ...;

  if ...
   return ...;

  return 8250_rx_chars(...);
}

Then in this patch only a couple of lines will be added to the end of
the function.

  if (...)
   return ...

-- 
With Best Regards,
Andy Shevchenko