linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] tty: TX helpers
@ 2022-04-11 10:54 Jiri Slaby
  2022-04-11 10:54 ` [PATCH 1/3] tty: serial: introduce uart_port_tx{,_limit}() helpers Jiri Slaby
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Jiri Slaby @ 2022-04-11 10:54 UTC (permalink / raw)
  To: gregkh
  Cc: linux-serial, linux-kernel, Jiri Slaby, Tobias Klauser,
	Richard Genoud, Nicolas Ferre, Alexandre Belloni, Claudiu Beznea,
	Vladimir Zapolskiy, Liviu Dudau, Sudeep Holla, Lorenzo Pieralisi,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Andreas Färber, Manivannan Sadhasivam,
	Russell King, Florian Fainelli, bcm-kernel-feedback-list,
	Pali Rohár, Kevin Cernekee, Palmer Dabbelt, Paul Walmsley,
	Orson Zhai, Baolin Wang, Chunyan Zhang, Patrice Chotard,
	linux-riscv

First, this series depends on "tty: various cleanups"
(20220411104506.8990-1-jslaby@suse.cz).

This series introduces uart_port_tx{,_limit}() TX helpers. See PATCH
1/3 for the details. Comments welcome.

Then it switches drivers to use them. First, uart_port_tx() in 2/3 and
then uart_port_tx_limit() in 3/3.

The diffstat of patches 2+3 is as follows:
 27 files changed, 330 insertions(+), 883 deletions(-)
which appears to be nice.

Cc: Tobias Klauser <tklauser@distanz.ch>
Cc: Richard Genoud <richard.genoud@gmail.com>
Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
Cc: Vladimir Zapolskiy <vz@mleia.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: bcm-kernel-feedback-list@broadcom.com
Cc: "Pali Rohár" <pali@kernel.org>
Cc: Kevin Cernekee <cernekee@gmail.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Orson Zhai <orsonzhai@gmail.com>
Cc: Baolin Wang <baolin.wang7@gmail.com>
Cc: Chunyan Zhang <zhang.lyra@gmail.com>
Cc: Patrice Chotard <patrice.chotard@foss.st.com>
Cc: linux-riscv@lists.infradead.org

Jiri Slaby (3):
  tty: serial: introduce uart_port_tx{,_limit}() helpers
  tty: serial: use uart_port_tx() helper
  tty: serial: use uart_port_tx_limit() helper

 Documentation/driver-api/serial/driver.rst | 28 +++++++++++
 drivers/tty/serial/21285.c                 | 40 ++++++----------
 drivers/tty/serial/altera_jtaguart.c       | 43 +++++------------
 drivers/tty/serial/altera_uart.c           | 47 ++++++-------------
 drivers/tty/serial/amba-pl010.c            | 40 +++-------------
 drivers/tty/serial/apbuart.c               | 37 +++------------
 drivers/tty/serial/atmel_serial.c          | 37 ++++++---------
 drivers/tty/serial/bcm63xx_uart.c          | 48 ++++---------------
 drivers/tty/serial/fsl_lpuart.c            | 49 +++++++-------------
 drivers/tty/serial/lantiq.c                | 50 +++++++-------------
 drivers/tty/serial/lpc32xx_hs.c            | 53 +++++++--------------
 drivers/tty/serial/mcf.c                   | 35 ++++++--------
 drivers/tty/serial/mpc52xx_uart.c          | 54 +++++++---------------
 drivers/tty/serial/mps2-uart.c             | 44 ++++++------------
 drivers/tty/serial/mux.c                   | 48 ++++++-------------
 drivers/tty/serial/mvebu-uart.c            | 47 ++++++-------------
 drivers/tty/serial/mxs-auart.c             | 43 ++++++++---------
 drivers/tty/serial/omap-serial.c           | 53 ++++++---------------
 drivers/tty/serial/owl-uart.c              | 47 ++++++-------------
 drivers/tty/serial/pxa.c                   | 43 ++++-------------
 drivers/tty/serial/rp2.c                   | 36 ++++-----------
 drivers/tty/serial/sa1100.c                | 50 ++++++++------------
 drivers/tty/serial/serial_core.c           | 53 +++++++++++++++++++++
 drivers/tty/serial/serial_txx9.c           | 40 +++-------------
 drivers/tty/serial/sifive.c                | 48 +++----------------
 drivers/tty/serial/sprd_serial.c           | 41 +++-------------
 drivers/tty/serial/st-asc.c                | 51 +++-----------------
 drivers/tty/serial/vr41xx_siu.c            | 42 +++--------------
 drivers/tty/serial/vt8500_serial.c         | 47 ++++++-------------
 include/linux/serial_core.h                |  9 ++++
 30 files changed, 420 insertions(+), 883 deletions(-)

-- 
2.35.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] tty: serial: introduce uart_port_tx{,_limit}() helpers
  2022-04-11 10:54 [PATCH 0/3] tty: TX helpers Jiri Slaby
@ 2022-04-11 10:54 ` Jiri Slaby
  2022-04-14 16:32   ` Greg KH
  2022-04-11 10:54 ` [PATCH 2/3] tty: serial: use uart_port_tx() helper Jiri Slaby
  2022-04-11 10:54 ` [PATCH 3/3] tty: serial: use uart_port_tx_limit() helper Jiri Slaby
  2 siblings, 1 reply; 10+ messages in thread
From: Jiri Slaby @ 2022-04-11 10:54 UTC (permalink / raw)
  To: gregkh; +Cc: linux-serial, linux-kernel, Jiri Slaby

Many serial drivers do the same thing:
* send x_char if set
* keep sending from the xmit circular buffer until either
  - the loop reaches the end of the xmit buffer
  - TX is stopped
  - HW fifo is full
* check for pending characters and:
  - wake up tty writers to fill for more data into xmit buffer
  - stop TX if there is nothing in the xmit buffer

The only differences are:
* how to write the character to the HW fifo
* the check of the end condition:
  - is the HW fifo full?
  - is limit of the written characters reached?

So unify the above into two helpers:
* uart_port_tx_limit() -- the generic one, it performs the above taking
  into account the written characters limit
* uart_port_tx() -- calls the above with ~0 as the limit. So it only
  checks the HW fullness.

We need three more hooks in struct uart_ops for all this to work:
* tx_ready() -- returns true if HW can accept more data.
* put_char() -- write a character to the device.
* tx_done() -- when the write loop is done, perform arbitrary action
  before potential invocation of ops->stop_tx() happens.

NOTE1: Maybe the three hooks in uart_ops above are overkill. We can
instead pass pointers to the three functions directly to the new helpers
as they are not used elsewhere. Similar to uart_console_write() and its
putchar().

NOTE2: These two new helper functions call the hooks per every character
processed. I was unable to measure any difference, provided most time is
spent by readb (or alike) in the hooks themselves.  First, LTO might
help to eliminate these explicit calls (we might need NOTE1 to be
implemented for this to be true). Second, if this turns out to be a
problem, we can introduce a macro to build the helper in the driver's
code instead of serial_core. That is, similar to wait_event().

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 Documentation/driver-api/serial/driver.rst | 28 ++++++++++++
 drivers/tty/serial/serial_core.c           | 53 ++++++++++++++++++++++
 include/linux/serial_core.h                |  9 ++++
 3 files changed, 90 insertions(+)

diff --git a/Documentation/driver-api/serial/driver.rst b/Documentation/driver-api/serial/driver.rst
index 06ec04ba086f..7dc3791addeb 100644
--- a/Documentation/driver-api/serial/driver.rst
+++ b/Documentation/driver-api/serial/driver.rst
@@ -80,6 +80,34 @@ hardware.
 
 	This call must not sleep
 
+  tx_ready(port)
+	The driver returns true if the HW can accept more data to be sent.
+
+	Locking: port->lock taken.
+
+	Interrupts: locally disabled.
+
+	This call must not sleep.
+
+  put_char(port, ch)
+	The driver is asked to write ch to the device.
+
+	Locking: port->lock taken.
+
+	Interrupts: locally disabled.
+
+	This call must not sleep.
+
+  tx_done(port)
+	When the write loop is done, the driver can perform arbitrary action
+	here before potential invocation of ops->stop_tx() happens.
+
+	Locking: port->lock taken.
+
+	Interrupts: locally disabled.
+
+	This call must not sleep.
+
   set_mctrl(port, mctrl)
 	This function sets the modem control lines for port described
 	by 'port' to the state described by mctrl.  The relevant bits
diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 6a8963caf954..1be14e90066c 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -107,6 +107,59 @@ void uart_write_wakeup(struct uart_port *port)
 }
 EXPORT_SYMBOL(uart_write_wakeup);
 
+static bool uart_port_tx_always_ready(struct uart_port *port)
+{
+	return true;
+}
+
+/**
+ * uart_port_tx_limit -- transmit helper for uart_port
+ * @port: from which port to transmit
+ * @count: limit count
+ *
+ * uart_port_tx_limit() transmits characters from the xmit buffer to the
+ * hardware using @uart_port::ops::put_char(). It does so until @count
+ * characters are sent and while @uart_port::ops::tx_ready() still returns
+ * non-zero (if non-NULL).
+ *
+ * Return: number of characters in the xmit buffer when done.
+ */
+unsigned int uart_port_tx_limit(struct uart_port *port, unsigned int count)
+{
+	struct circ_buf *xmit = &port->state->xmit;
+	bool (*tx_ready)(struct uart_port *) = port->ops->tx_ready ? :
+		uart_port_tx_always_ready;
+	unsigned int pending;
+
+	for (; count && tx_ready(port); count--, port->icount.tx++) {
+		if (port->x_char) {
+			port->ops->put_char(port, port->x_char);
+			port->x_char = 0;
+			continue;
+		}
+
+		if (uart_circ_empty(xmit) || uart_tx_stopped(port))
+			break;
+
+		port->ops->put_char(port, xmit->buf[xmit->tail]);
+		xmit->tail = (xmit->tail + 1) % UART_XMIT_SIZE;
+	}
+
+	if (port->ops->tx_done)
+		port->ops->tx_done(port);
+
+	pending = uart_circ_chars_pending(xmit);
+	if (pending < WAKEUP_CHARS) {
+		uart_write_wakeup(port);
+
+		if (pending == 0)
+			port->ops->stop_tx(port);
+	}
+
+	return pending;
+}
+EXPORT_SYMBOL(uart_port_tx_limit);
+
 static void uart_stop(struct tty_struct *tty)
 {
 	struct uart_state *state = tty->driver_data;
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index d4828e69087a..c990a20b9989 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -37,6 +37,9 @@ struct gpio_desc;
  */
 struct uart_ops {
 	unsigned int	(*tx_empty)(struct uart_port *);
+	bool		(*tx_ready)(struct uart_port *);
+	void		(*put_char)(struct uart_port *, unsigned char ch);
+	void		(*tx_done)(struct uart_port *);
 	void		(*set_mctrl)(struct uart_port *, unsigned int mctrl);
 	unsigned int	(*get_mctrl)(struct uart_port *);
 	void		(*stop_tx)(struct uart_port *);
@@ -321,6 +324,12 @@ struct uart_driver {
 };
 
 void uart_write_wakeup(struct uart_port *port);
+unsigned int uart_port_tx_limit(struct uart_port *port, unsigned int count);
+
+static inline unsigned int uart_port_tx(struct uart_port *port)
+{
+	return uart_port_tx_limit(port, ~0U);
+}
 
 /*
  * Baud rate helpers.
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] tty: serial: use uart_port_tx() helper
  2022-04-11 10:54 [PATCH 0/3] tty: TX helpers Jiri Slaby
  2022-04-11 10:54 ` [PATCH 1/3] tty: serial: introduce uart_port_tx{,_limit}() helpers Jiri Slaby
@ 2022-04-11 10:54 ` Jiri Slaby
  2022-04-11 10:54 ` [PATCH 3/3] tty: serial: use uart_port_tx_limit() helper Jiri Slaby
  2 siblings, 0 replies; 10+ messages in thread
From: Jiri Slaby @ 2022-04-11 10:54 UTC (permalink / raw)
  To: gregkh
  Cc: linux-serial, linux-kernel, Jiri Slaby, Tobias Klauser,
	Richard Genoud, Nicolas Ferre, Alexandre Belloni, Claudiu Beznea,
	Vladimir Zapolskiy, Liviu Dudau, Sudeep Holla, Lorenzo Pieralisi,
	Shawn Guo, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	NXP Linux Team, Andreas Färber, Manivannan Sadhasivam

uart_port_tx() is a new helper to send characters to the device. Use
it in these drivers.

It means we have to define two new uart hooks: tx_ready() and put_char()
to do the real job now.

Cc: Tobias Klauser <tklauser@distanz.ch>
Cc: Richard Genoud <richard.genoud@gmail.com>
Cc: Nicolas Ferre <nicolas.ferre@microchip.com>
Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
Cc: Claudiu Beznea <claudiu.beznea@microchip.com>
Cc: Vladimir Zapolskiy <vz@mleia.com>
Cc: Liviu Dudau <liviu.dudau@arm.com>
Cc: Sudeep Holla <sudeep.holla@arm.com>
Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: "Andreas Färber" <afaerber@suse.de>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---
 drivers/tty/serial/altera_uart.c   | 47 ++++++++------------------
 drivers/tty/serial/atmel_serial.c  | 37 +++++++++-----------
 drivers/tty/serial/fsl_lpuart.c    | 49 ++++++++++-----------------
 drivers/tty/serial/lantiq.c        | 50 ++++++++++-----------------
 drivers/tty/serial/lpc32xx_hs.c    | 53 +++++++++--------------------
 drivers/tty/serial/mcf.c           | 35 +++++++------------
 drivers/tty/serial/mpc52xx_uart.c  | 54 +++++++++---------------------
 drivers/tty/serial/mps2-uart.c     | 44 ++++++++----------------
 drivers/tty/serial/mxs-auart.c     | 43 +++++++++++-------------
 drivers/tty/serial/owl-uart.c      | 47 +++++++-------------------
 drivers/tty/serial/sa1100.c        | 50 +++++++++++----------------
 drivers/tty/serial/vt8500_serial.c | 47 ++++++++------------------
 12 files changed, 187 insertions(+), 369 deletions(-)

diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index 8b749ed557c6..bfbfcafff4cc 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -97,6 +97,17 @@ static unsigned int altera_uart_tx_empty(struct uart_port *port)
 		ALTERA_UART_STATUS_TMT_MSK) ? TIOCSER_TEMT : 0;
 }
 
+static bool altera_uart_tx_ready(struct uart_port *port)
+{
+	return altera_uart_readl(port, ALTERA_UART_STATUS_REG) &
+		ALTERA_UART_STATUS_TRDY_MSK;
+}
+
+static void altera_uart_put_char(struct uart_port *port, unsigned char ch)
+{
+	altera_uart_writel(port, ch, ALTERA_UART_TXDATA_REG);
+}
+
 static unsigned int altera_uart_get_mctrl(struct uart_port *port)
 {
 	struct altera_uart *pp = container_of(port, struct altera_uart, port);
@@ -246,38 +257,6 @@ static void altera_uart_rx_chars(struct altera_uart *pp)
 	tty_flip_buffer_push(&port->state->port);
 }
 
-static void altera_uart_tx_chars(struct altera_uart *pp)
-{
-	struct uart_port *port = &pp->port;
-	struct circ_buf *xmit = &port->state->xmit;
-
-	if (port->x_char) {
-		/* Send special char - probably flow control */
-		altera_uart_writel(port, port->x_char, ALTERA_UART_TXDATA_REG);
-		port->x_char = 0;
-		port->icount.tx++;
-		return;
-	}
-
-	while (altera_uart_readl(port, ALTERA_UART_STATUS_REG) &
-	       ALTERA_UART_STATUS_TRDY_MSK) {
-		if (xmit->head == xmit->tail)
-			break;
-		altera_uart_writel(port, xmit->buf[xmit->tail],
-		       ALTERA_UART_TXDATA_REG);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	if (xmit->head == xmit->tail) {
-		pp->imr &= ~ALTERA_UART_CONTROL_TRDY_MSK;
-		altera_uart_update_ctrl_reg(pp);
-	}
-}
-
 static irqreturn_t altera_uart_interrupt(int irq, void *data)
 {
 	struct uart_port *port = data;
@@ -290,7 +269,7 @@ static irqreturn_t altera_uart_interrupt(int irq, void *data)
 	if (isr & ALTERA_UART_STATUS_RRDY_MSK)
 		altera_uart_rx_chars(pp);
 	if (isr & ALTERA_UART_STATUS_TRDY_MSK)
-		altera_uart_tx_chars(pp);
+		uart_port_tx(&pp->port);
 	spin_unlock(&port->lock);
 
 	return IRQ_RETVAL(isr);
@@ -414,6 +393,8 @@ static void altera_uart_poll_put_char(struct uart_port *port, unsigned char c)
  */
 static const struct uart_ops altera_uart_ops = {
 	.tx_empty	= altera_uart_tx_empty,
+	.tx_ready	= altera_uart_tx_ready,
+	.put_char	= altera_uart_put_char,
 	.get_mctrl	= altera_uart_get_mctrl,
 	.set_mctrl	= altera_uart_set_mctrl,
 	.start_tx	= altera_uart_start_tx,
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 3a45e4fc7993..bf4c3892bbbc 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -462,6 +462,16 @@ static u_int atmel_tx_empty(struct uart_port *port)
 		0;
 }
 
+static bool atmel_uart_tx_ready(struct uart_port *port)
+{
+	return atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY;
+}
+
+static void atmel_put_char(struct uart_port *port, unsigned char ch)
+{
+	atmel_uart_write_char(port, ch);
+}
+
 /*
  * Set state of the modem control output lines
  */
@@ -822,30 +832,11 @@ static void atmel_rx_chars(struct uart_port *port)
  */
 static void atmel_tx_chars(struct uart_port *port)
 {
-	struct circ_buf *xmit = &port->state->xmit;
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+	unsigned int pending;
 
-	if (port->x_char &&
-	    (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY)) {
-		atmel_uart_write_char(port, port->x_char);
-		port->icount.tx++;
-		port->x_char = 0;
-	}
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port))
-		return;
-
-	while (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY) {
-		atmel_uart_write_char(port, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	if (!uart_circ_empty(xmit)) {
+	pending = uart_port_tx(port);
+	if (pending) {
 		/* we still have characters to transmit, so we should continue
 		 * transmitting them when TX is ready, regardless of
 		 * mode or duplexity
@@ -2451,6 +2442,8 @@ static void atmel_poll_put_char(struct uart_port *port, unsigned char ch)
 
 static const struct uart_ops atmel_pops = {
 	.tx_empty	= atmel_tx_empty,
+	.tx_ready	= atmel_uart_tx_ready,
+	.put_char	= atmel_put_char,
 	.set_mctrl	= atmel_set_mctrl,
 	.get_mctrl	= atmel_get_mctrl,
 	.stop_tx	= atmel_stop_tx,
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index 87789872f400..a696fc4a5968 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -741,36 +741,6 @@ static int lpuart32_poll_get_char(struct uart_port *port)
 }
 #endif
 
-static inline void lpuart_transmit_buffer(struct lpuart_port *sport)
-{
-	struct circ_buf *xmit = &sport->port.state->xmit;
-
-	if (sport->port.x_char) {
-		writeb(sport->port.x_char, sport->port.membase + UARTDR);
-		sport->port.icount.tx++;
-		sport->port.x_char = 0;
-		return;
-	}
-
-	if (lpuart_stopped_or_empty(&sport->port)) {
-		lpuart_stop_tx(&sport->port);
-		return;
-	}
-
-	while (!uart_circ_empty(xmit) &&
-		(readb(sport->port.membase + UARTTCFIFO) < sport->txfifo_size)) {
-		writeb(xmit->buf[xmit->tail], sport->port.membase + UARTDR);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		sport->port.icount.tx++;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(&sport->port);
-
-	if (uart_circ_empty(xmit))
-		lpuart_stop_tx(&sport->port);
-}
-
 static inline void lpuart32_transmit_buffer(struct lpuart_port *sport)
 {
 	struct circ_buf *xmit = &sport->port.state->xmit;
@@ -821,7 +791,7 @@ static void lpuart_start_tx(struct uart_port *port)
 			lpuart_dma_tx(sport);
 	} else {
 		if (readb(port->membase + UARTSR1) & UARTSR1_TDRE)
-			lpuart_transmit_buffer(sport);
+			uart_port_tx(&sport->port);
 	}
 }
 
@@ -875,10 +845,23 @@ static unsigned int lpuart32_tx_empty(struct uart_port *port)
 	return 0;
 }
 
+static bool lpuart_tx_ready(struct uart_port *port)
+{
+	struct lpuart_port *sport = container_of(port, struct lpuart_port,
+			port);
+
+	return readb(port->membase + UARTTCFIFO) < sport->txfifo_size;
+}
+
+static void lpuart_put_char(struct uart_port *port, unsigned char ch)
+{
+	writeb(ch, port->membase + UARTDR);
+}
+
 static void lpuart_txint(struct lpuart_port *sport)
 {
 	spin_lock(&sport->port.lock);
-	lpuart_transmit_buffer(sport);
+	uart_port_tx(&sport->port);
 	spin_unlock(&sport->port.lock);
 }
 
@@ -2284,6 +2267,8 @@ static int lpuart_verify_port(struct uart_port *port, struct serial_struct *ser)
 
 static const struct uart_ops lpuart_pops = {
 	.tx_empty	= lpuart_tx_empty,
+	.tx_ready	= lpuart_tx_ready,
+	.put_char	= lpuart_put_char,
 	.set_mctrl	= lpuart_set_mctrl,
 	.get_mctrl	= lpuart_get_mctrl,
 	.stop_tx	= lpuart_stop_tx,
diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
index a3120c3347dd..11d2519ce231 100644
--- a/drivers/tty/serial/lantiq.c
+++ b/drivers/tty/serial/lantiq.c
@@ -95,7 +95,6 @@
 #define ASCFSTAT_TXFREEMASK	0x3F000000
 #define ASCFSTAT_TXFREEOFF	24
 
-static void lqasc_tx_chars(struct uart_port *port);
 static struct ltq_uart_port *lqasc_port[MAXPORTS];
 static struct uart_driver lqasc_reg;
 
@@ -146,7 +145,7 @@ lqasc_start_tx(struct uart_port *port)
 	struct ltq_uart_port *ltq_port = to_ltq_uart_port(port);
 
 	spin_lock_irqsave(&ltq_port->lock, flags);
-	lqasc_tx_chars(port);
+	uart_port_tx(port);
 	spin_unlock_irqrestore(&ltq_port->lock, flags);
 	return;
 }
@@ -219,37 +218,6 @@ lqasc_rx_chars(struct uart_port *port)
 	return 0;
 }
 
-static void
-lqasc_tx_chars(struct uart_port *port)
-{
-	struct circ_buf *xmit = &port->state->xmit;
-	if (uart_tx_stopped(port)) {
-		lqasc_stop_tx(port);
-		return;
-	}
-
-	while (((__raw_readl(port->membase + LTQ_ASC_FSTAT) &
-		ASCFSTAT_TXFREEMASK) >> ASCFSTAT_TXFREEOFF) != 0) {
-		if (port->x_char) {
-			writeb(port->x_char, port->membase + LTQ_ASC_TBUF);
-			port->icount.tx++;
-			port->x_char = 0;
-			continue;
-		}
-
-		if (uart_circ_empty(xmit))
-			break;
-
-		writeb(port->state->xmit.buf[port->state->xmit.tail],
-			port->membase + LTQ_ASC_TBUF);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-}
-
 static irqreturn_t
 lqasc_tx_int(int irq, void *_port)
 {
@@ -327,6 +295,20 @@ lqasc_tx_empty(struct uart_port *port)
 	return status ? 0 : TIOCSER_TEMT;
 }
 
+static bool
+lqasc_tx_ready(struct uart_port *port)
+{
+	u32 fstat = __raw_readl(port->membase + LTQ_ASC_FSTAT);
+
+	return (fstat & ASCFSTAT_TXFREEMASK) >> ASCFSTAT_TXFREEOFF;
+}
+
+static void
+lqasc_put_char(struct uart_port *port, unsigned char ch)
+{
+	writeb(ch, port->membase + LTQ_ASC_TBUF);
+}
+
 static unsigned int
 lqasc_get_mctrl(struct uart_port *port)
 {
@@ -580,6 +562,8 @@ lqasc_verify_port(struct uart_port *port,
 
 static const struct uart_ops lqasc_pops = {
 	.tx_empty =	lqasc_tx_empty,
+	.tx_ready =	lqasc_tx_ready,
+	.put_char =	lqasc_put_char,
 	.set_mctrl =	lqasc_set_mctrl,
 	.get_mctrl =	lqasc_get_mctrl,
 	.stop_tx =	lqasc_stop_tx,
diff --git a/drivers/tty/serial/lpc32xx_hs.c b/drivers/tty/serial/lpc32xx_hs.c
index 93140cac1ca1..0bcd4a43b746 100644
--- a/drivers/tty/serial/lpc32xx_hs.c
+++ b/drivers/tty/serial/lpc32xx_hs.c
@@ -276,41 +276,6 @@ static void __serial_lpc32xx_rx(struct uart_port *port)
 	tty_flip_buffer_push(tport);
 }
 
-static void serial_lpc32xx_stop_tx(struct uart_port *port);
-
-static void __serial_lpc32xx_tx(struct uart_port *port)
-{
-	struct circ_buf *xmit = &port->state->xmit;
-
-	if (port->x_char) {
-		writel((u32)port->x_char, LPC32XX_HSUART_FIFO(port->membase));
-		port->icount.tx++;
-		port->x_char = 0;
-		return;
-	}
-
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port))
-		goto exit_tx;
-
-	/* Transfer data */
-	while (LPC32XX_HSU_TX_LEV(readl(
-		LPC32XX_HSUART_LEVEL(port->membase))) < 64) {
-		writel((u32) xmit->buf[xmit->tail],
-		       LPC32XX_HSUART_FIFO(port->membase));
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-exit_tx:
-	if (uart_circ_empty(xmit))
-		serial_lpc32xx_stop_tx(port);
-}
-
 static irqreturn_t serial_lpc32xx_interrupt(int irq, void *dev_id)
 {
 	struct uart_port *port = dev_id;
@@ -349,7 +314,7 @@ static irqreturn_t serial_lpc32xx_interrupt(int irq, void *dev_id)
 	/* Transmit data request? */
 	if ((status & LPC32XX_HSU_TX_INT) && (!uart_tx_stopped(port))) {
 		writel(LPC32XX_HSU_TX_INT, LPC32XX_HSUART_IIR(port->membase));
-		__serial_lpc32xx_tx(port);
+		uart_port_tx(port);
 	}
 
 	spin_unlock(&port->lock);
@@ -368,6 +333,18 @@ static unsigned int serial_lpc32xx_tx_empty(struct uart_port *port)
 	return ret;
 }
 
+static bool serial_lpc32xx_tx_ready(struct uart_port *port)
+{
+	u32 level = readl(LPC32XX_HSUART_LEVEL(port->membase));
+
+	return LPC32XX_HSU_TX_LEV(level) < 64;
+}
+
+static void serial_lpc32xx_put_char(struct uart_port *port, unsigned char ch)
+{
+	writel(ch, LPC32XX_HSUART_FIFO(port->membase));
+}
+
 /* port->lock held by caller.  */
 static void serial_lpc32xx_set_mctrl(struct uart_port *port,
 				     unsigned int mctrl)
@@ -397,7 +374,7 @@ static void serial_lpc32xx_start_tx(struct uart_port *port)
 {
 	u32 tmp;
 
-	__serial_lpc32xx_tx(port);
+	uart_port_tx(port);
 	tmp = readl(LPC32XX_HSUART_CTRL(port->membase));
 	tmp |= LPC32XX_HSU_TX_INT_EN;
 	writel(tmp, LPC32XX_HSUART_CTRL(port->membase));
@@ -607,6 +584,8 @@ static int serial_lpc32xx_verify_port(struct uart_port *port,
 
 static const struct uart_ops serial_lpc32xx_pops = {
 	.tx_empty	= serial_lpc32xx_tx_empty,
+	.tx_ready	= serial_lpc32xx_tx_ready,
+	.put_char	= serial_lpc32xx_put_char,
 	.set_mctrl	= serial_lpc32xx_set_mctrl,
 	.get_mctrl	= serial_lpc32xx_get_mctrl,
 	.stop_tx	= serial_lpc32xx_stop_tx,
diff --git a/drivers/tty/serial/mcf.c b/drivers/tty/serial/mcf.c
index 2aec62b5d6c4..f2c3a9586bc5 100644
--- a/drivers/tty/serial/mcf.c
+++ b/drivers/tty/serial/mcf.c
@@ -63,6 +63,16 @@ static unsigned int mcf_tx_empty(struct uart_port *port)
 		TIOCSER_TEMT : 0;
 }
 
+static bool mcf_tx_ready(struct uart_port *port)
+{
+	return readb(port->membase + MCFUART_USR) & MCFUART_USR_TXREADY;
+}
+
+static void mcf_put_char(struct uart_port *port, unsigned char ch)
+{
+	writeb(ch, port->membase + MCFUART_UTB);
+}
+
 /****************************************************************************/
 
 static unsigned int mcf_get_mctrl(struct uart_port *port)
@@ -327,29 +337,8 @@ static void mcf_rx_chars(struct mcf_uart *pp)
 static void mcf_tx_chars(struct mcf_uart *pp)
 {
 	struct uart_port *port = &pp->port;
-	struct circ_buf *xmit = &port->state->xmit;
-
-	if (port->x_char) {
-		/* Send special char - probably flow control */
-		writeb(port->x_char, port->membase + MCFUART_UTB);
-		port->x_char = 0;
-		port->icount.tx++;
-		return;
-	}
-
-	while (readb(port->membase + MCFUART_USR) & MCFUART_USR_TXREADY) {
-		if (uart_circ_empty(xmit))
-			break;
-		writeb(xmit->buf[xmit->tail], port->membase + MCFUART_UTB);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE -1);
-		port->icount.tx++;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
 
-	if (uart_circ_empty(xmit)) {
-		mcf_stop_tx(port);
+	if (!uart_port_tx(port)) {
 		/* Disable TX to negate RTS automatically */
 		if (port->rs485.flags & SER_RS485_ENABLED)
 			writeb(MCFUART_UCR_TXDISABLE,
@@ -460,6 +449,8 @@ static int mcf_config_rs485(struct uart_port *port, struct serial_rs485 *rs485)
  */
 static const struct uart_ops mcf_uart_ops = {
 	.tx_empty	= mcf_tx_empty,
+	.tx_ready	= mcf_tx_ready,
+	.put_char	= mcf_put_char,
 	.get_mctrl	= mcf_get_mctrl,
 	.set_mctrl	= mcf_set_mctrl,
 	.start_tx	= mcf_start_tx,
diff --git a/drivers/tty/serial/mpc52xx_uart.c b/drivers/tty/serial/mpc52xx_uart.c
index c4b590dd6f23..1073164d1b01 100644
--- a/drivers/tty/serial/mpc52xx_uart.c
+++ b/drivers/tty/serial/mpc52xx_uart.c
@@ -1043,6 +1043,18 @@ mpc52xx_uart_tx_empty(struct uart_port *port)
 	return psc_ops->tx_empty(port) ? TIOCSER_TEMT : 0;
 }
 
+static bool
+mpc52xx_uart_tx_ready(struct uart_port *port)
+{
+	return psc_ops->raw_tx_rdy(port);
+}
+
+static void
+mpc52xx_uart_put_char(struct uart_port *port, unsigned char ch)
+{
+	psc_ops->write_char(port, ch);
+}
+
 static void
 mpc52xx_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
@@ -1336,9 +1348,10 @@ mpc52xx_uart_verify_port(struct uart_port *port, struct serial_struct *ser)
 	return 0;
 }
 
-
 static const struct uart_ops mpc52xx_uart_ops = {
 	.tx_empty	= mpc52xx_uart_tx_empty,
+	.tx_ready	= mpc52xx_uart_tx_ready,
+	.put_char	= mpc52xx_uart_put_char,
 	.set_mctrl	= mpc52xx_uart_set_mctrl,
 	.get_mctrl	= mpc52xx_uart_get_mctrl,
 	.stop_tx	= mpc52xx_uart_stop_tx,
@@ -1423,45 +1436,10 @@ mpc52xx_uart_int_rx_chars(struct uart_port *port)
 	return psc_ops->raw_rx_rdy(port);
 }
 
-static inline int
+static inline unsigned int
 mpc52xx_uart_int_tx_chars(struct uart_port *port)
 {
-	struct circ_buf *xmit = &port->state->xmit;
-
-	/* Process out of band chars */
-	if (port->x_char) {
-		psc_ops->write_char(port, port->x_char);
-		port->icount.tx++;
-		port->x_char = 0;
-		return 1;
-	}
-
-	/* Nothing to do ? */
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
-		mpc52xx_uart_stop_tx(port);
-		return 0;
-	}
-
-	/* Send chars */
-	while (psc_ops->raw_tx_rdy(port)) {
-		psc_ops->write_char(port, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	}
-
-	/* Wake up */
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	/* Maybe we're done after all */
-	if (uart_circ_empty(xmit)) {
-		mpc52xx_uart_stop_tx(port);
-		return 0;
-	}
-
-	return 1;
+	return uart_port_tx(port);
 }
 
 static irqreturn_t
diff --git a/drivers/tty/serial/mps2-uart.c b/drivers/tty/serial/mps2-uart.c
index 5e9429dcc51f..bbb2000032da 100644
--- a/drivers/tty/serial/mps2-uart.c
+++ b/drivers/tty/serial/mps2-uart.c
@@ -109,6 +109,16 @@ static unsigned int mps2_uart_tx_empty(struct uart_port *port)
 	return (status & UARTn_STATE_TX_FULL) ? 0 : TIOCSER_TEMT;
 }
 
+static bool mps2_uart_tx_ready(struct uart_port *port)
+{
+	return mps2_uart_tx_empty(port);
+}
+
+static void mps2_uart_put_char(struct uart_port *port, unsigned char ch)
+{
+	mps2_uart_write8(port, ch, UARTn_DATA);
+}
+
 static void mps2_uart_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
 }
@@ -127,33 +137,6 @@ static void mps2_uart_stop_tx(struct uart_port *port)
 	mps2_uart_write8(port, control, UARTn_CTRL);
 }
 
-static void mps2_uart_tx_chars(struct uart_port *port)
-{
-	struct circ_buf *xmit = &port->state->xmit;
-
-	while (!(mps2_uart_read8(port, UARTn_STATE) & UARTn_STATE_TX_FULL)) {
-		if (port->x_char) {
-			mps2_uart_write8(port, port->x_char, UARTn_DATA);
-			port->x_char = 0;
-			port->icount.tx++;
-			continue;
-		}
-
-		if (uart_circ_empty(xmit) || uart_tx_stopped(port))
-			break;
-
-		mps2_uart_write8(port, xmit->buf[xmit->tail], UARTn_DATA);
-		xmit->tail = (xmit->tail + 1) % UART_XMIT_SIZE;
-		port->icount.tx++;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	if (uart_circ_empty(xmit))
-		mps2_uart_stop_tx(port);
-}
-
 static void mps2_uart_start_tx(struct uart_port *port)
 {
 	u8 control = mps2_uart_read8(port, UARTn_CTRL);
@@ -168,8 +151,7 @@ static void mps2_uart_start_tx(struct uart_port *port)
 	 * write buffer in one go, the TX IRQ should assert, at which
 	 * point we switch to fully interrupt-driven TX.
 	 */
-
-	mps2_uart_tx_chars(port);
+	uart_port_tx(port);
 }
 
 static void mps2_uart_stop_rx(struct uart_port *port)
@@ -228,7 +210,7 @@ static irqreturn_t mps2_uart_txirq(int irq, void *data)
 	spin_lock(&port->lock);
 
 	mps2_uart_write8(port, UARTn_INT_TX, UARTn_INT);
-	mps2_uart_tx_chars(port);
+	uart_port_tx(port);
 
 	spin_unlock(&port->lock);
 
@@ -413,6 +395,8 @@ static int mps2_uart_verify_port(struct uart_port *port, struct serial_struct *s
 
 static const struct uart_ops mps2_uart_pops = {
 	.tx_empty = mps2_uart_tx_empty,
+	.tx_ready = mps2_uart_tx_ready,
+	.put_char = mps2_uart_put_char,
 	.set_mctrl = mps2_uart_set_mctrl,
 	.get_mctrl = mps2_uart_get_mctrl,
 	.stop_tx = mps2_uart_stop_tx,
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 1944daf8593a..884c47db3e50 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -603,31 +603,10 @@ static void mxs_auart_tx_chars(struct mxs_auart_port *s)
 		return;
 	}
 
-
-	while (!(mxs_read(s, REG_STAT) & AUART_STAT_TXFF)) {
-		if (s->port.x_char) {
-			s->port.icount.tx++;
-			mxs_write(s->port.x_char, s, REG_DATA);
-			s->port.x_char = 0;
-			continue;
-		}
-		if (!uart_circ_empty(xmit) && !uart_tx_stopped(&s->port)) {
-			s->port.icount.tx++;
-			mxs_write(xmit->buf[xmit->tail], s, REG_DATA);
-			xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		} else
-			break;
-	}
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(&s->port);
-
-	if (uart_circ_empty(&(s->port.state->xmit)))
-		mxs_clr(AUART_INTR_TXIEN, s, REG_INTR);
-	else
+	if (uart_port_tx(&s->port))
 		mxs_set(AUART_INTR_TXIEN, s, REG_INTR);
-
-	if (uart_tx_stopped(&s->port))
-		mxs_auart_stop_tx(&s->port);
+	else
+		mxs_clr(AUART_INTR_TXIEN, s, REG_INTR);
 }
 
 static void mxs_auart_rx_char(struct mxs_auart_port *s)
@@ -1248,6 +1227,20 @@ static unsigned int mxs_auart_tx_empty(struct uart_port *u)
 	return 0;
 }
 
+static bool mxs_auart_tx_ready(struct uart_port *u)
+{
+	struct mxs_auart_port *s = to_auart_port(u);
+
+	return !(mxs_read(s, REG_STAT) & AUART_STAT_TXFF);
+}
+
+static void mxs_auart_put_char(struct uart_port *u, unsigned char ch)
+{
+	struct mxs_auart_port *s = to_auart_port(u);
+
+	mxs_write(ch, s, REG_DATA);
+}
+
 static void mxs_auart_start_tx(struct uart_port *u)
 {
 	struct mxs_auart_port *s = to_auart_port(u);
@@ -1284,6 +1277,8 @@ static void mxs_auart_break_ctl(struct uart_port *u, int ctl)
 
 static const struct uart_ops mxs_auart_ops = {
 	.tx_empty       = mxs_auart_tx_empty,
+	.tx_ready	= mxs_auart_tx_ready,
+	.put_char	= mxs_auart_put_char,
 	.start_tx       = mxs_auart_start_tx,
 	.stop_tx	= mxs_auart_stop_tx,
 	.stop_rx	= mxs_auart_stop_rx,
diff --git a/drivers/tty/serial/owl-uart.c b/drivers/tty/serial/owl-uart.c
index 5ff7c89aeb06..0d584b528ee0 100644
--- a/drivers/tty/serial/owl-uart.c
+++ b/drivers/tty/serial/owl-uart.c
@@ -135,6 +135,16 @@ static unsigned int owl_uart_tx_empty(struct uart_port *port)
 	return ret;
 }
 
+static bool owl_uart_tx_ready(struct uart_port *port)
+{
+	return !(owl_uart_read(port, OWL_UART_STAT) & OWL_UART_STAT_TFFU);
+}
+
+static void owl_uart_put_char(struct uart_port *port, unsigned char ch)
+{
+	owl_uart_write(port, ch, OWL_UART_TXDAT);
+}
+
 static void owl_uart_stop_rx(struct uart_port *port)
 {
 	u32 val;
@@ -179,39 +189,6 @@ static void owl_uart_start_tx(struct uart_port *port)
 	owl_uart_write(port, val, OWL_UART_CTL);
 }
 
-static void owl_uart_send_chars(struct uart_port *port)
-{
-	struct circ_buf *xmit = &port->state->xmit;
-	unsigned int ch;
-
-	if (port->x_char) {
-		while (!(owl_uart_read(port, OWL_UART_STAT) & OWL_UART_STAT_TFFU))
-			cpu_relax();
-		owl_uart_write(port, port->x_char, OWL_UART_TXDAT);
-		port->icount.tx++;
-		port->x_char = 0;
-	}
-
-	if (uart_tx_stopped(port))
-		return;
-
-	while (!(owl_uart_read(port, OWL_UART_STAT) & OWL_UART_STAT_TFFU)) {
-		if (uart_circ_empty(xmit))
-			break;
-
-		ch = xmit->buf[xmit->tail];
-		owl_uart_write(port, ch, OWL_UART_TXDAT);
-		xmit->tail = (xmit->tail + 1) & (SERIAL_XMIT_SIZE - 1);
-		port->icount.tx++;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	if (uart_circ_empty(xmit))
-		owl_uart_stop_tx(port);
-}
-
 static void owl_uart_receive_chars(struct uart_port *port)
 {
 	u32 stat, val;
@@ -264,7 +241,7 @@ static irqreturn_t owl_uart_irq(int irq, void *dev_id)
 		owl_uart_receive_chars(port);
 
 	if (stat & OWL_UART_STAT_TIP)
-		owl_uart_send_chars(port);
+		uart_port_tx(port);
 
 	stat = owl_uart_read(port, OWL_UART_STAT);
 	stat |= OWL_UART_STAT_RIP | OWL_UART_STAT_TIP;
@@ -497,6 +474,8 @@ static const struct uart_ops owl_uart_ops = {
 	.set_mctrl = owl_uart_set_mctrl,
 	.get_mctrl = owl_uart_get_mctrl,
 	.tx_empty = owl_uart_tx_empty,
+	.tx_ready = owl_uart_tx_ready,
+	.put_char = owl_uart_put_char,
 	.start_tx = owl_uart_start_tx,
 	.stop_rx = owl_uart_stop_rx,
 	.stop_tx = owl_uart_stop_tx,
diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
index 5fe6cccfc1ae..ac26ff4358b6 100644
--- a/drivers/tty/serial/sa1100.c
+++ b/drivers/tty/serial/sa1100.c
@@ -228,43 +228,13 @@ sa1100_rx_chars(struct sa1100_port *sport)
 
 static void sa1100_tx_chars(struct sa1100_port *sport)
 {
-	struct circ_buf *xmit = &sport->port.state->xmit;
-
-	if (sport->port.x_char) {
-		UART_PUT_CHAR(sport, sport->port.x_char);
-		sport->port.icount.tx++;
-		sport->port.x_char = 0;
-		return;
-	}
-
 	/*
 	 * Check the modem control lines before
 	 * transmitting anything.
 	 */
 	sa1100_mctrl_check(sport);
 
-	if (uart_circ_empty(xmit) || uart_tx_stopped(&sport->port)) {
-		sa1100_stop_tx(&sport->port);
-		return;
-	}
-
-	/*
-	 * Tried using FIFO (not checking TNF) for fifo fill:
-	 * still had the '4 bytes repeated' problem.
-	 */
-	while (UART_GET_UTSR1(sport) & UTSR1_TNF) {
-		UART_PUT_CHAR(sport, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		sport->port.icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(&sport->port);
-
-	if (uart_circ_empty(xmit))
-		sa1100_stop_tx(&sport->port);
+	uart_port_tx(&sport->port);
 }
 
 static irqreturn_t sa1100_int(int irq, void *dev_id)
@@ -317,6 +287,22 @@ static unsigned int sa1100_tx_empty(struct uart_port *port)
 	return UART_GET_UTSR1(sport) & UTSR1_TBY ? 0 : TIOCSER_TEMT;
 }
 
+static bool sa1100_tx_ready(struct uart_port *port)
+{
+	struct sa1100_port *sport =
+		container_of(port, struct sa1100_port, port);
+
+	return UART_GET_UTSR1(sport) & UTSR1_TNF;
+}
+
+static void sa1100_put_char(struct uart_port *port, unsigned char ch)
+{
+	struct sa1100_port *sport =
+		container_of(port, struct sa1100_port, port);
+
+	UART_PUT_CHAR(sport, ch);
+}
+
 static unsigned int sa1100_get_mctrl(struct uart_port *port)
 {
 	struct sa1100_port *sport =
@@ -588,6 +574,8 @@ sa1100_verify_port(struct uart_port *port, struct serial_struct *ser)
 
 static struct uart_ops sa1100_pops = {
 	.tx_empty	= sa1100_tx_empty,
+	.tx_ready	= sa1100_tx_ready,
+	.put_char	= sa1100_put_char,
 	.set_mctrl	= sa1100_set_mctrl,
 	.get_mctrl	= sa1100_get_mctrl,
 	.stop_tx	= sa1100_stop_tx,
diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
index 6f08136ce78a..c4cbf6e3b247 100644
--- a/drivers/tty/serial/vt8500_serial.c
+++ b/drivers/tty/serial/vt8500_serial.c
@@ -187,37 +187,6 @@ static void handle_rx(struct uart_port *port)
 	tty_flip_buffer_push(tport);
 }
 
-static void handle_tx(struct uart_port *port)
-{
-	struct circ_buf *xmit = &port->state->xmit;
-
-	if (port->x_char) {
-		writeb(port->x_char, port->membase + VT8500_TXFIFO);
-		port->icount.tx++;
-		port->x_char = 0;
-	}
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
-		vt8500_stop_tx(port);
-		return;
-	}
-
-	while ((vt8500_read(port, VT8500_URFIDX) & 0x1f) < 16) {
-		if (uart_circ_empty(xmit))
-			break;
-
-		writeb(xmit->buf[xmit->tail], port->membase + VT8500_TXFIFO);
-
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	if (uart_circ_empty(xmit))
-		vt8500_stop_tx(port);
-}
-
 static void vt8500_start_tx(struct uart_port *port)
 {
 	struct vt8500_port *vt8500_port = container_of(port,
@@ -226,7 +195,7 @@ static void vt8500_start_tx(struct uart_port *port)
 
 	vt8500_port->ier &= ~TX_FIFO_INTS;
 	vt8500_write(port, vt8500_port->ier, VT8500_URIER);
-	handle_tx(port);
+	uart_port_tx(port);
 	vt8500_port->ier |= TX_FIFO_INTS;
 	vt8500_write(port, vt8500_port->ier, VT8500_URIER);
 }
@@ -251,7 +220,7 @@ static irqreturn_t vt8500_irq(int irq, void *dev_id)
 	if (isr & RX_FIFO_INTS)
 		handle_rx(port);
 	if (isr & TX_FIFO_INTS)
-		handle_tx(port);
+		uart_port_tx(port);
 	if (isr & TCTS)
 		handle_delta_cts(port);
 
@@ -266,6 +235,16 @@ static unsigned int vt8500_tx_empty(struct uart_port *port)
 						TIOCSER_TEMT : 0;
 }
 
+static bool vt8500_tx_ready(struct uart_port *port)
+{
+	return vt8500_tx_empty(port);
+}
+
+static void vt8500_put_char(struct uart_port *port, unsigned char ch)
+{
+	writeb(ch, port->membase + VT8500_TXFIFO);
+}
+
 static unsigned int vt8500_get_mctrl(struct uart_port *port)
 {
 	unsigned int usr;
@@ -580,6 +559,8 @@ static void vt8500_put_poll_char(struct uart_port *port, unsigned char c)
 
 static const struct uart_ops vt8500_uart_pops = {
 	.tx_empty	= vt8500_tx_empty,
+	.tx_ready	= vt8500_tx_ready,
+	.put_char	= vt8500_put_char,
 	.set_mctrl	= vt8500_set_mctrl,
 	.get_mctrl	= vt8500_get_mctrl,
 	.stop_tx	= vt8500_stop_tx,
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] tty: serial: use uart_port_tx_limit() helper
  2022-04-11 10:54 [PATCH 0/3] tty: TX helpers Jiri Slaby
  2022-04-11 10:54 ` [PATCH 1/3] tty: serial: introduce uart_port_tx{,_limit}() helpers Jiri Slaby
  2022-04-11 10:54 ` [PATCH 2/3] tty: serial: use uart_port_tx() helper Jiri Slaby
@ 2022-04-11 10:54 ` Jiri Slaby
  2022-04-11 11:51   ` Pali Rohár
  2 siblings, 1 reply; 10+ messages in thread
From: Jiri Slaby @ 2022-04-11 10:54 UTC (permalink / raw)
  To: gregkh
  Cc: linux-serial, linux-kernel, Jiri Slaby, Russell King,
	Florian Fainelli, bcm-kernel-feedback-list, Pali Rohár,
	Kevin Cernekee, Palmer Dabbelt, Paul Walmsley, Orson Zhai,
	Baolin Wang, Chunyan Zhang, Patrice Chotard, linux-riscv

uart_port_tx_limit() is a new helper to send characters to the device.
Use it in these drivers.

It means we have to define two new uart hooks: tx_ready() and put_char()
to do the real job now.

And mux.c also needs to define tx_done(). But I'm not sure if the driver
really wants to wait for all the characters to dismiss from the HW fifo
at this code point. Hence I marked this as FIXME.

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Florian Fainelli <f.fainelli@gmail.com>
Cc: bcm-kernel-feedback-list@broadcom.com
Cc: "Pali Rohár" <pali@kernel.org>
Cc: Kevin Cernekee <cernekee@gmail.com>
Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>
Cc: Orson Zhai <orsonzhai@gmail.com>
Cc: Baolin Wang <baolin.wang7@gmail.com>
Cc: Chunyan Zhang <zhang.lyra@gmail.com>
Cc: Patrice Chotard <patrice.chotard@foss.st.com>
Cc: linux-riscv@lists.infradead.org
---
 drivers/tty/serial/21285.c           | 40 +++++++--------------
 drivers/tty/serial/altera_jtaguart.c | 43 ++++++----------------
 drivers/tty/serial/amba-pl010.c      | 40 ++++-----------------
 drivers/tty/serial/apbuart.c         | 37 ++++---------------
 drivers/tty/serial/bcm63xx_uart.c    | 48 ++++++-------------------
 drivers/tty/serial/mux.c             | 48 ++++++++-----------------
 drivers/tty/serial/mvebu-uart.c      | 47 +++++++-----------------
 drivers/tty/serial/omap-serial.c     | 53 +++++++---------------------
 drivers/tty/serial/pxa.c             | 43 +++++-----------------
 drivers/tty/serial/rp2.c             | 36 ++++++-------------
 drivers/tty/serial/serial_txx9.c     | 40 ++++-----------------
 drivers/tty/serial/sifive.c          | 48 ++++---------------------
 drivers/tty/serial/sprd_serial.c     | 41 ++++-----------------
 drivers/tty/serial/st-asc.c          | 51 ++++----------------------
 drivers/tty/serial/vr41xx_siu.c      | 42 ++++------------------
 15 files changed, 143 insertions(+), 514 deletions(-)

diff --git a/drivers/tty/serial/21285.c b/drivers/tty/serial/21285.c
index 7520cc02fd4d..12d9bdcd67b8 100644
--- a/drivers/tty/serial/21285.c
+++ b/drivers/tty/serial/21285.c
@@ -154,35 +154,9 @@ static irqreturn_t serial21285_rx_chars(int irq, void *dev_id)
 static irqreturn_t serial21285_tx_chars(int irq, void *dev_id)
 {
 	struct uart_port *port = dev_id;
-	struct circ_buf *xmit = &port->state->xmit;
-	int count = 256;
-
-	if (port->x_char) {
-		*CSR_UARTDR = port->x_char;
-		port->icount.tx++;
-		port->x_char = 0;
-		goto out;
-	}
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
-		serial21285_stop_tx(port);
-		goto out;
-	}
-
-	do {
-		*CSR_UARTDR = xmit->buf[xmit->tail];
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	} while (--count > 0 && !(*CSR_UARTFLG & 0x20));
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
 
-	if (uart_circ_empty(xmit))
-		serial21285_stop_tx(port);
+	uart_port_tx_limit(port, 256);
 
- out:
 	return IRQ_HANDLED;
 }
 
@@ -191,6 +165,16 @@ static unsigned int serial21285_tx_empty(struct uart_port *port)
 	return (*CSR_UARTFLG & 8) ? 0 : TIOCSER_TEMT;
 }
 
+static bool serial21285_uart_tx_ready(struct uart_port *port)
+{
+	return !(*CSR_UARTFLG & 0x20);
+}
+
+static void serial21285_put_char(struct uart_port *port, unsigned char ch)
+{
+	*CSR_UARTDR = ch;
+}
+
 /* no modem control lines */
 static unsigned int serial21285_get_mctrl(struct uart_port *port)
 {
@@ -372,6 +356,8 @@ static int serial21285_verify_port(struct uart_port *port, struct serial_struct
 
 static const struct uart_ops serial21285_ops = {
 	.tx_empty	= serial21285_tx_empty,
+	.tx_ready	= serial21285_uart_tx_ready,
+	.put_char	= serial21285_put_char,
 	.get_mctrl	= serial21285_get_mctrl,
 	.set_mctrl	= serial21285_set_mctrl,
 	.stop_tx	= serial21285_stop_tx,
diff --git a/drivers/tty/serial/altera_jtaguart.c b/drivers/tty/serial/altera_jtaguart.c
index cb791c5149a3..144cb4ab337b 100644
--- a/drivers/tty/serial/altera_jtaguart.c
+++ b/drivers/tty/serial/altera_jtaguart.c
@@ -65,6 +65,11 @@ static unsigned int altera_jtaguart_tx_empty(struct uart_port *port)
 		ALTERA_JTAGUART_CONTROL_WSPACE_MSK) ? TIOCSER_TEMT : 0;
 }
 
+static void altera_jtaguart_put_char(struct uart_port *port, unsigned char ch)
+{
+	writel(ch, port->membase + ALTERA_JTAGUART_DATA_REG);
+}
+
 static unsigned int altera_jtaguart_get_mctrl(struct uart_port *port)
 {
 	return TIOCM_CAR | TIOCM_DSR | TIOCM_CTS;
@@ -137,39 +142,12 @@ static void altera_jtaguart_rx_chars(struct altera_jtaguart *pp)
 static void altera_jtaguart_tx_chars(struct altera_jtaguart *pp)
 {
 	struct uart_port *port = &pp->port;
-	struct circ_buf *xmit = &port->state->xmit;
-	unsigned int pending, count;
-
-	if (port->x_char) {
-		/* Send special char - probably flow control */
-		writel(port->x_char, port->membase + ALTERA_JTAGUART_DATA_REG);
-		port->x_char = 0;
-		port->icount.tx++;
-		return;
-	}
-
-	pending = uart_circ_chars_pending(xmit);
-	if (pending > 0) {
-		count = (readl(port->membase + ALTERA_JTAGUART_CONTROL_REG) &
-				ALTERA_JTAGUART_CONTROL_WSPACE_MSK) >>
-			ALTERA_JTAGUART_CONTROL_WSPACE_OFF;
-		if (count > pending)
-			count = pending;
-		if (count > 0) {
-			pending -= count;
-			while (count--) {
-				writel(xmit->buf[xmit->tail],
-				       port->membase + ALTERA_JTAGUART_DATA_REG);
-				xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-				port->icount.tx++;
-			}
-			if (pending < WAKEUP_CHARS)
-				uart_write_wakeup(port);
-		}
-	}
+	unsigned int space;
 
-	if (pending == 0)
-		altera_jtaguart_stop_tx(port);
+	space = (readl(port->membase + ALTERA_JTAGUART_CONTROL_REG) &
+			ALTERA_JTAGUART_CONTROL_WSPACE_MSK) >>
+		ALTERA_JTAGUART_CONTROL_WSPACE_OFF;
+	uart_port_tx_limit(port, space);
 }
 
 static irqreturn_t altera_jtaguart_interrupt(int irq, void *data)
@@ -274,6 +252,7 @@ static int altera_jtaguart_verify_port(struct uart_port *port,
  */
 static const struct uart_ops altera_jtaguart_ops = {
 	.tx_empty	= altera_jtaguart_tx_empty,
+	.put_char       = altera_jtaguart_put_char,
 	.get_mctrl	= altera_jtaguart_get_mctrl,
 	.set_mctrl	= altera_jtaguart_set_mctrl,
 	.start_tx	= altera_jtaguart_start_tx,
diff --git a/drivers/tty/serial/amba-pl010.c b/drivers/tty/serial/amba-pl010.c
index fae0b581ff42..1d73e1ba318b 100644
--- a/drivers/tty/serial/amba-pl010.c
+++ b/drivers/tty/serial/amba-pl010.c
@@ -162,38 +162,6 @@ static void pl010_rx_chars(struct uart_port *port)
 	tty_flip_buffer_push(&port->state->port);
 }
 
-static void pl010_tx_chars(struct uart_port *port)
-{
-	struct circ_buf *xmit = &port->state->xmit;
-	int count;
-
-	if (port->x_char) {
-		writel(port->x_char, port->membase + UART01x_DR);
-		port->icount.tx++;
-		port->x_char = 0;
-		return;
-	}
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
-		pl010_stop_tx(port);
-		return;
-	}
-
-	count = port->fifosize >> 1;
-	do {
-		writel(xmit->buf[xmit->tail], port->membase + UART01x_DR);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	} while (--count > 0);
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	if (uart_circ_empty(xmit))
-		pl010_stop_tx(port);
-}
-
 static void pl010_modem_status(struct uart_amba_port *uap)
 {
 	struct uart_port *port = &uap->port;
@@ -238,7 +206,7 @@ static irqreturn_t pl010_int(int irq, void *dev_id)
 			if (status & UART010_IIR_MIS)
 				pl010_modem_status(uap);
 			if (status & UART010_IIR_TIS)
-				pl010_tx_chars(port);
+				uart_port_tx_limit(port, port->fifosize >> 1);
 
 			if (pass_counter-- == 0)
 				break;
@@ -261,6 +229,11 @@ static unsigned int pl010_tx_empty(struct uart_port *port)
 	return status & UART01x_FR_BUSY ? 0 : TIOCSER_TEMT;
 }
 
+static void pl010_put_char(struct uart_port *port, unsigned char ch)
+{
+	writel(ch, port->membase + UART01x_DR);
+}
+
 static unsigned int pl010_get_mctrl(struct uart_port *port)
 {
 	unsigned int result = 0;
@@ -529,6 +502,7 @@ static int pl010_verify_port(struct uart_port *port, struct serial_struct *ser)
 
 static const struct uart_ops amba_pl010_pops = {
 	.tx_empty	= pl010_tx_empty,
+	.put_char	= pl010_put_char,
 	.set_mctrl	= pl010_set_mctrl,
 	.get_mctrl	= pl010_get_mctrl,
 	.stop_tx	= pl010_stop_tx,
diff --git a/drivers/tty/serial/apbuart.c b/drivers/tty/serial/apbuart.c
index 9ef82d870ff2..a31bd5849c76 100644
--- a/drivers/tty/serial/apbuart.c
+++ b/drivers/tty/serial/apbuart.c
@@ -122,36 +122,7 @@ static void apbuart_rx_chars(struct uart_port *port)
 
 static void apbuart_tx_chars(struct uart_port *port)
 {
-	struct circ_buf *xmit = &port->state->xmit;
-	int count;
-
-	if (port->x_char) {
-		UART_PUT_CHAR(port, port->x_char);
-		port->icount.tx++;
-		port->x_char = 0;
-		return;
-	}
-
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
-		apbuart_stop_tx(port);
-		return;
-	}
-
-	/* amba: fill FIFO */
-	count = port->fifosize >> 1;
-	do {
-		UART_PUT_CHAR(port, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	} while (--count > 0);
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	if (uart_circ_empty(xmit))
-		apbuart_stop_tx(port);
+	uart_port_tx_limit(port, port->fifosize >> 1);
 }
 
 static irqreturn_t apbuart_int(int irq, void *dev_id)
@@ -178,6 +149,11 @@ static unsigned int apbuart_tx_empty(struct uart_port *port)
 	return status & UART_STATUS_THE ? TIOCSER_TEMT : 0;
 }
 
+static void apbuart_put_char(struct uart_port *port, unsigned char ch)
+{
+	UART_PUT_CHAR(port, ch);
+}
+
 static unsigned int apbuart_get_mctrl(struct uart_port *port)
 {
 	/* The GRLIB APBUART handles flow control in hardware */
@@ -322,6 +298,7 @@ static int apbuart_verify_port(struct uart_port *port,
 
 static const struct uart_ops grlib_apbuart_ops = {
 	.tx_empty = apbuart_tx_empty,
+	.put_char = apbuart_put_char,
 	.set_mctrl = apbuart_set_mctrl,
 	.get_mctrl = apbuart_get_mctrl,
 	.stop_tx = apbuart_stop_tx,
diff --git a/drivers/tty/serial/bcm63xx_uart.c b/drivers/tty/serial/bcm63xx_uart.c
index 53b43174aa40..29f31286ff54 100644
--- a/drivers/tty/serial/bcm63xx_uart.c
+++ b/drivers/tty/serial/bcm63xx_uart.c
@@ -93,6 +93,11 @@ static unsigned int bcm_uart_tx_empty(struct uart_port *port)
 	return (val & UART_IR_STAT(UART_IR_TXEMPTY)) ? 1 : 0;
 }
 
+static void bcm_uart_put_char(struct uart_port *port, unsigned char ch)
+{
+	bcm_uart_writel(port, ch, UART_FIFO_REG);
+}
+
 /*
  * serial core request to set RTS and DTR pin state and loopback mode
  */
@@ -303,53 +308,19 @@ static void bcm_uart_do_rx(struct uart_port *port)
  */
 static void bcm_uart_do_tx(struct uart_port *port)
 {
-	struct circ_buf *xmit;
-	unsigned int val, max_count;
-
-	if (port->x_char) {
-		bcm_uart_writel(port, port->x_char, UART_FIFO_REG);
-		port->icount.tx++;
-		port->x_char = 0;
-		return;
-	}
-
-	if (uart_tx_stopped(port)) {
-		bcm_uart_stop_tx(port);
-		return;
-	}
-
-	xmit = &port->state->xmit;
-	if (uart_circ_empty(xmit))
-		goto txq_empty;
+	unsigned int val, pending;
 
 	val = bcm_uart_readl(port, UART_MCTL_REG);
 	val = (val & UART_MCTL_TXFIFOFILL_MASK) >> UART_MCTL_TXFIFOFILL_SHIFT;
-	max_count = port->fifosize - val;
-
-	while (max_count--) {
-		unsigned int c;
-
-		c = xmit->buf[xmit->tail];
-		bcm_uart_writel(port, c, UART_FIFO_REG);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	}
 
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	if (uart_circ_empty(xmit))
-		goto txq_empty;
-	return;
+	pending = uart_port_tx_limit(port, port->fifosize - val);
+	if (pending)
+		return;
 
-txq_empty:
 	/* nothing to send, disable transmit interrupt */
 	val = bcm_uart_readl(port, UART_IR_REG);
 	val &= ~UART_TX_INT_MASK;
 	bcm_uart_writel(port, val, UART_IR_REG);
-	return;
 }
 
 /*
@@ -629,6 +600,7 @@ static int bcm_uart_verify_port(struct uart_port *port,
 /* serial core callbacks */
 static const struct uart_ops bcm_uart_ops = {
 	.tx_empty	= bcm_uart_tx_empty,
+	.put_char	= bcm_uart_put_char,
 	.get_mctrl	= bcm_uart_get_mctrl,
 	.set_mctrl	= bcm_uart_set_mctrl,
 	.start_tx	= bcm_uart_start_tx,
diff --git a/drivers/tty/serial/mux.c b/drivers/tty/serial/mux.c
index 643dfbcc43f9..e86718788b49 100644
--- a/drivers/tty/serial/mux.c
+++ b/drivers/tty/serial/mux.c
@@ -106,6 +106,18 @@ static unsigned int mux_tx_empty(struct uart_port *port)
 	return UART_GET_FIFO_CNT(port) ? 0 : TIOCSER_TEMT;
 } 
 
+static void mux_put_char(struct uart_port *port, unsigned char ch)
+{
+	UART_PUT_CHAR(port, ch);
+}
+
+static void mux_tx_done(struct uart_port *port)
+{
+	/* FIXME js: really needs to wait? */
+	while (UART_GET_FIFO_CNT(port))
+		udelay(1);
+}
+
 /**
  * mux_set_mctrl - Set the current state of the modem control inputs.
  * @ports: Ptr to the uart_port.
@@ -180,39 +192,7 @@ static void mux_break_ctl(struct uart_port *port, int break_state)
  */
 static void mux_write(struct uart_port *port)
 {
-	int count;
-	struct circ_buf *xmit = &port->state->xmit;
-
-	if(port->x_char) {
-		UART_PUT_CHAR(port, port->x_char);
-		port->icount.tx++;
-		port->x_char = 0;
-		return;
-	}
-
-	if(uart_circ_empty(xmit) || uart_tx_stopped(port)) {
-		mux_stop_tx(port);
-		return;
-	}
-
-	count = (port->fifosize) - UART_GET_FIFO_CNT(port);
-	do {
-		UART_PUT_CHAR(port, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-		if(uart_circ_empty(xmit))
-			break;
-
-	} while(--count > 0);
-
-	while(UART_GET_FIFO_CNT(port)) 
-		udelay(1);
-
-	if(uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	if (uart_circ_empty(xmit))
-		mux_stop_tx(port);
+	uart_port_tx_limit(port, port->fifosize - UART_GET_FIFO_CNT(port));
 }
 
 /**
@@ -421,6 +401,8 @@ static struct console mux_console = {
 
 static const struct uart_ops mux_pops = {
 	.tx_empty =		mux_tx_empty,
+	.put_char =		mux_put_char,
+	.tx_done =		mux_tx_done,
 	.set_mctrl =		mux_set_mctrl,
 	.get_mctrl =		mux_get_mctrl,
 	.stop_tx =		mux_stop_tx,
diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index 0429c2a54290..3d07ab9eb15e 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -194,6 +194,16 @@ static unsigned int mvebu_uart_tx_empty(struct uart_port *port)
 	return (st & STAT_TX_EMP) ? TIOCSER_TEMT : 0;
 }
 
+static bool mvebu_uart_tx_ready(struct uart_port *port)
+{
+	return !(readl(port->membase + UART_STAT) & STAT_TX_FIFO_FUL);
+}
+
+static void mvebu_uart_put_char(struct uart_port *port, unsigned char ch)
+{
+	writel(ch, port->membase + UART_TSH(port));
+}
+
 static unsigned int mvebu_uart_get_mctrl(struct uart_port *port)
 {
 	return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
@@ -324,40 +334,7 @@ static void mvebu_uart_rx_chars(struct uart_port *port, unsigned int status)
 
 static void mvebu_uart_tx_chars(struct uart_port *port, unsigned int status)
 {
-	struct circ_buf *xmit = &port->state->xmit;
-	unsigned int count;
-	unsigned int st;
-
-	if (port->x_char) {
-		writel(port->x_char, port->membase + UART_TSH(port));
-		port->icount.tx++;
-		port->x_char = 0;
-		return;
-	}
-
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
-		mvebu_uart_stop_tx(port);
-		return;
-	}
-
-	for (count = 0; count < port->fifosize; count++) {
-		writel(xmit->buf[xmit->tail], port->membase + UART_TSH(port));
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-
-		if (uart_circ_empty(xmit))
-			break;
-
-		st = readl(port->membase + UART_STAT);
-		if (st & STAT_TX_FIFO_FUL)
-			break;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	if (uart_circ_empty(xmit))
-		mvebu_uart_stop_tx(port);
+	uart_port_tx_limit(port, port->fifosize);
 }
 
 static irqreturn_t mvebu_uart_isr(int irq, void *dev_id)
@@ -654,6 +631,8 @@ static void mvebu_uart_put_poll_char(struct uart_port *port, unsigned char c)
 
 static const struct uart_ops mvebu_uart_ops = {
 	.tx_empty	= mvebu_uart_tx_empty,
+	.tx_ready	= mvebu_uart_tx_ready,
+	.put_char	= mvebu_uart_put_char,
 	.set_mctrl	= mvebu_uart_set_mctrl,
 	.get_mctrl	= mvebu_uart_get_mctrl,
 	.stop_tx	= mvebu_uart_stop_tx,
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 8d5ffa196097..a6446bef49f1 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -336,45 +336,6 @@ static void serial_omap_stop_rx(struct uart_port *port)
 	serial_out(up, UART_IER, up->ier);
 }
 
-static void transmit_chars(struct uart_omap_port *up, unsigned int lsr)
-{
-	struct circ_buf *xmit = &up->port.state->xmit;
-	int count;
-
-	if (up->port.x_char) {
-		serial_out(up, UART_TX, up->port.x_char);
-		up->port.icount.tx++;
-		up->port.x_char = 0;
-		if ((up->port.rs485.flags & SER_RS485_ENABLED) &&
-		    !(up->port.rs485.flags & SER_RS485_RX_DURING_TX))
-			up->rs485_tx_filter_count++;
-
-		return;
-	}
-	if (uart_circ_empty(xmit) || uart_tx_stopped(&up->port)) {
-		serial_omap_stop_tx(&up->port);
-		return;
-	}
-	count = up->port.fifosize / 4;
-	do {
-		serial_out(up, UART_TX, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		up->port.icount.tx++;
-		if ((up->port.rs485.flags & SER_RS485_ENABLED) &&
-		    !(up->port.rs485.flags & SER_RS485_RX_DURING_TX))
-			up->rs485_tx_filter_count++;
-
-		if (uart_circ_empty(xmit))
-			break;
-	} while (--count > 0);
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(&up->port);
-
-	if (uart_circ_empty(xmit))
-		serial_omap_stop_tx(&up->port);
-}
-
 static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)
 {
 	if (!(up->ier & UART_IER_THRI)) {
@@ -572,7 +533,7 @@ static irqreturn_t serial_omap_irq(int irq, void *dev_id)
 			check_modem_status(up);
 			break;
 		case UART_IIR_THRI:
-			transmit_chars(up, lsr);
+			uart_port_tx_limit(&up->port, up->port.fifosize / 4);
 			break;
 		case UART_IIR_RX_TIMEOUT:
 		case UART_IIR_RDI:
@@ -613,6 +574,17 @@ static unsigned int serial_omap_tx_empty(struct uart_port *port)
 	return ret;
 }
 
+static void serial_omap_put_char(struct uart_port *port, unsigned char ch)
+{
+	struct uart_omap_port *up = to_uart_omap_port(port);
+
+	serial_out(up, UART_TX, ch);
+
+	if ((up->port.rs485.flags & SER_RS485_ENABLED) &&
+	    !(up->port.rs485.flags & SER_RS485_RX_DURING_TX))
+		up->rs485_tx_filter_count++;
+}
+
 static unsigned int serial_omap_get_mctrl(struct uart_port *port)
 {
 	struct uart_omap_port *up = to_uart_omap_port(port);
@@ -1369,6 +1341,7 @@ serial_omap_config_rs485(struct uart_port *port, struct serial_rs485 *rs485)
 
 static const struct uart_ops serial_omap_pops = {
 	.tx_empty	= serial_omap_tx_empty,
+	.put_char	= serial_omap_put_char,
 	.set_mctrl	= serial_omap_set_mctrl,
 	.get_mctrl	= serial_omap_get_mctrl,
 	.stop_tx	= serial_omap_stop_tx,
diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index e80ba8e10407..182bf2de5e29 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -171,39 +171,6 @@ static inline void receive_chars(struct uart_pxa_port *up, int *status)
 	serial_out(up, UART_IER, up->ier);
 }
 
-static void transmit_chars(struct uart_pxa_port *up)
-{
-	struct circ_buf *xmit = &up->port.state->xmit;
-	int count;
-
-	if (up->port.x_char) {
-		serial_out(up, UART_TX, up->port.x_char);
-		up->port.icount.tx++;
-		up->port.x_char = 0;
-		return;
-	}
-	if (uart_circ_empty(xmit) || uart_tx_stopped(&up->port)) {
-		serial_pxa_stop_tx(&up->port);
-		return;
-	}
-
-	count = up->port.fifosize / 2;
-	do {
-		serial_out(up, UART_TX, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		up->port.icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	} while (--count > 0);
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(&up->port);
-
-
-	if (uart_circ_empty(xmit))
-		serial_pxa_stop_tx(&up->port);
-}
-
 static void serial_pxa_start_tx(struct uart_port *port)
 {
 	struct uart_pxa_port *up = (struct uart_pxa_port *)port;
@@ -253,7 +220,7 @@ static inline irqreturn_t serial_pxa_irq(int irq, void *dev_id)
 		receive_chars(up, &lsr);
 	check_modem_status(up);
 	if (lsr & UART_LSR_THRE)
-		transmit_chars(up);
+		uart_port_tx_limit(&up->port, up->port.fifosize / 2);
 	spin_unlock(&up->port.lock);
 	return IRQ_HANDLED;
 }
@@ -271,6 +238,13 @@ static unsigned int serial_pxa_tx_empty(struct uart_port *port)
 	return ret;
 }
 
+static void serial_pxa_put_char(struct uart_port *port, unsigned char ch)
+{
+	struct uart_pxa_port *up = (struct uart_pxa_port *)port;
+
+	serial_out(up, UART_TX, ch);
+}
+
 static unsigned int serial_pxa_get_mctrl(struct uart_port *port)
 {
 	struct uart_pxa_port *up = (struct uart_pxa_port *)port;
@@ -742,6 +716,7 @@ static struct console serial_pxa_console = {
 
 static const struct uart_ops serial_pxa_pops = {
 	.tx_empty	= serial_pxa_tx_empty,
+	.put_char	= serial_pxa_put_char,
 	.set_mctrl	= serial_pxa_set_mctrl,
 	.get_mctrl	= serial_pxa_get_mctrl,
 	.stop_tx	= serial_pxa_stop_tx,
diff --git a/drivers/tty/serial/rp2.c b/drivers/tty/serial/rp2.c
index 6689d8add8f7..18725fe9028c 100644
--- a/drivers/tty/serial/rp2.c
+++ b/drivers/tty/serial/rp2.c
@@ -283,6 +283,13 @@ static unsigned int rp2_uart_tx_empty(struct uart_port *port)
 	return tx_fifo_bytes ? 0 : TIOCSER_TEMT;
 }
 
+static void rp2_put_char(struct uart_port *port, unsigned char ch)
+{
+	struct rp2_uart_port *up = port_to_up(port);
+
+	writeb(ch, up->base + RP2_DATA_BYTE);
+}
+
 static unsigned int rp2_uart_get_mctrl(struct uart_port *port)
 {
 	struct rp2_uart_port *up = port_to_up(port);
@@ -428,32 +435,8 @@ static void rp2_rx_chars(struct rp2_uart_port *up)
 
 static void rp2_tx_chars(struct rp2_uart_port *up)
 {
-	u16 max_tx = FIFO_SIZE - readw(up->base + RP2_TX_FIFO_COUNT);
-	struct circ_buf *xmit = &up->port.state->xmit;
-
-	if (uart_tx_stopped(&up->port)) {
-		rp2_uart_stop_tx(&up->port);
-		return;
-	}
-
-	for (; max_tx != 0; max_tx--) {
-		if (up->port.x_char) {
-			writeb(up->port.x_char, up->base + RP2_DATA_BYTE);
-			up->port.x_char = 0;
-			up->port.icount.tx++;
-			continue;
-		}
-		if (uart_circ_empty(xmit)) {
-			rp2_uart_stop_tx(&up->port);
-			break;
-		}
-		writeb(xmit->buf[xmit->tail], up->base + RP2_DATA_BYTE);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		up->port.icount.tx++;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(&up->port);
+	uart_port_tx_limit(&up->port,
+			FIFO_SIZE - readw(up->base + RP2_TX_FIFO_COUNT));
 }
 
 static void rp2_ch_interrupt(struct rp2_uart_port *up)
@@ -575,6 +558,7 @@ static int rp2_uart_verify_port(struct uart_port *port,
 
 static const struct uart_ops rp2_uart_ops = {
 	.tx_empty	= rp2_uart_tx_empty,
+	.put_char	= rp2_put_char,
 	.set_mctrl	= rp2_uart_set_mctrl,
 	.get_mctrl	= rp2_uart_get_mctrl,
 	.stop_tx	= rp2_uart_stop_tx,
diff --git a/drivers/tty/serial/serial_txx9.c b/drivers/tty/serial/serial_txx9.c
index 2213e6b841d3..41b4b33b110e 100644
--- a/drivers/tty/serial/serial_txx9.c
+++ b/drivers/tty/serial/serial_txx9.c
@@ -319,38 +319,6 @@ receive_chars(struct uart_port *up, unsigned int *status)
 	*status = disr;
 }
 
-static inline void transmit_chars(struct uart_port *up)
-{
-	struct circ_buf *xmit = &up->state->xmit;
-	int count;
-
-	if (up->x_char) {
-		sio_out(up, TXX9_SITFIFO, up->x_char);
-		up->icount.tx++;
-		up->x_char = 0;
-		return;
-	}
-	if (uart_circ_empty(xmit) || uart_tx_stopped(up)) {
-		serial_txx9_stop_tx(up);
-		return;
-	}
-
-	count = TXX9_SIO_TX_FIFO;
-	do {
-		sio_out(up, TXX9_SITFIFO, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		up->icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	} while (--count > 0);
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(up);
-
-	if (uart_circ_empty(xmit))
-		serial_txx9_stop_tx(up);
-}
-
 static irqreturn_t serial_txx9_interrupt(int irq, void *dev_id)
 {
 	int pass_counter = 0;
@@ -371,7 +339,7 @@ static irqreturn_t serial_txx9_interrupt(int irq, void *dev_id)
 		if (status & TXX9_SIDISR_RDIS)
 			receive_chars(up, &status);
 		if (status & TXX9_SIDISR_TDIS)
-			transmit_chars(up);
+			uart_port_tx_limit(up, TXX9_SIO_TX_FIFO);
 		/* Clear TX/RX Int. Status */
 		sio_mask(up, TXX9_SIDISR,
 			 TXX9_SIDISR_TDIS | TXX9_SIDISR_RDIS |
@@ -397,6 +365,11 @@ static unsigned int serial_txx9_tx_empty(struct uart_port *up)
 	return ret;
 }
 
+static void serial_txx9_put_char(struct uart_port *up, unsigned char ch)
+{
+	sio_out(up, TXX9_SITFIFO, ch);
+}
+
 static unsigned int serial_txx9_get_mctrl(struct uart_port *up)
 {
 	unsigned int ret;
@@ -810,6 +783,7 @@ serial_txx9_type(struct uart_port *port)
 
 static const struct uart_ops serial_txx9_pops = {
 	.tx_empty	= serial_txx9_tx_empty,
+	.put_char	= serial_txx9_put_char,
 	.set_mctrl	= serial_txx9_set_mctrl,
 	.get_mctrl	= serial_txx9_get_mctrl,
 	.stop_tx	= serial_txx9_stop_tx,
diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
index f5ac14c384c4..7ec9ef732eb8 100644
--- a/drivers/tty/serial/sifive.c
+++ b/drivers/tty/serial/sifive.c
@@ -289,46 +289,6 @@ static void __ssp_transmit_char(struct sifive_serial_port *ssp, int ch)
 	__ssp_writel(ch, SIFIVE_SERIAL_TXDATA_OFFS, ssp);
 }
 
-/**
- * __ssp_transmit_chars() - enqueue multiple bytes onto the TX FIFO
- * @ssp: pointer to a struct sifive_serial_port
- *
- * Transfer up to a TX FIFO size's worth of characters from the Linux serial
- * transmit buffer to the SiFive UART TX FIFO.
- *
- * Context: Any context.  Expects @ssp->port.lock to be held by caller.
- */
-static void __ssp_transmit_chars(struct sifive_serial_port *ssp)
-{
-	struct circ_buf *xmit = &ssp->port.state->xmit;
-	int count;
-
-	if (ssp->port.x_char) {
-		__ssp_transmit_char(ssp, ssp->port.x_char);
-		ssp->port.icount.tx++;
-		ssp->port.x_char = 0;
-		return;
-	}
-	if (uart_circ_empty(xmit) || uart_tx_stopped(&ssp->port)) {
-		sifive_serial_stop_tx(&ssp->port);
-		return;
-	}
-	count = SIFIVE_TX_FIFO_DEPTH;
-	do {
-		__ssp_transmit_char(ssp, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		ssp->port.icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	} while (--count > 0);
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(&ssp->port);
-
-	if (uart_circ_empty(xmit))
-		sifive_serial_stop_tx(&ssp->port);
-}
-
 /**
  * __ssp_enable_txwm() - enable transmit watermark interrupts
  * @ssp: pointer to a struct sifive_serial_port
@@ -565,7 +525,7 @@ static irqreturn_t sifive_serial_irq(int irq, void *dev_id)
 	if (ip & SIFIVE_SERIAL_IP_RXWM_MASK)
 		__ssp_receive_chars(ssp);
 	if (ip & SIFIVE_SERIAL_IP_TXWM_MASK)
-		__ssp_transmit_chars(ssp);
+		uart_port_tx_limit(&ssp->port, SIFIVE_TX_FIFO_DEPTH);
 
 	spin_unlock(&ssp->port.lock);
 
@@ -577,6 +537,11 @@ static unsigned int sifive_serial_tx_empty(struct uart_port *port)
 	return TIOCSER_TEMT;
 }
 
+static void sifive_serial_put_char(struct uart_port *port, unsigned char ch)
+{
+	__ssp_transmit_char(port_to_sifive_serial_port(port), ch);
+}
+
 static unsigned int sifive_serial_get_mctrl(struct uart_port *port)
 {
 	return TIOCM_CAR | TIOCM_CTS | TIOCM_DSR;
@@ -905,6 +870,7 @@ static void __ssp_remove_console_port(struct sifive_serial_port *ssp)
 
 static const struct uart_ops sifive_serial_uops = {
 	.tx_empty	= sifive_serial_tx_empty,
+	.put_char	= sifive_serial_put_char,
 	.set_mctrl	= sifive_serial_set_mctrl,
 	.get_mctrl	= sifive_serial_get_mctrl,
 	.stop_tx	= sifive_serial_stop_tx,
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
index 4329b9c9cbf0..d92dc07f0ebb 100644
--- a/drivers/tty/serial/sprd_serial.c
+++ b/drivers/tty/serial/sprd_serial.c
@@ -154,6 +154,11 @@ static unsigned int sprd_tx_empty(struct uart_port *port)
 		return TIOCSER_TEMT;
 }
 
+static void sprd_put_char(struct uart_port *port, unsigned char ch)
+{
+	serial_out(port, SPRD_TXD, ch);
+}
+
 static unsigned int sprd_get_mctrl(struct uart_port *port)
 {
 	return TIOCM_DSR | TIOCM_CTS;
@@ -624,39 +629,6 @@ static inline void sprd_rx(struct uart_port *port)
 	tty_flip_buffer_push(tty);
 }
 
-static inline void sprd_tx(struct uart_port *port)
-{
-	struct circ_buf *xmit = &port->state->xmit;
-	int count;
-
-	if (port->x_char) {
-		serial_out(port, SPRD_TXD, port->x_char);
-		port->icount.tx++;
-		port->x_char = 0;
-		return;
-	}
-
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
-		sprd_stop_tx(port);
-		return;
-	}
-
-	count = THLD_TX_EMPTY;
-	do {
-		serial_out(port, SPRD_TXD, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	} while (--count > 0);
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	if (uart_circ_empty(xmit))
-		sprd_stop_tx(port);
-}
-
 /* this handles the interrupt from one port */
 static irqreturn_t sprd_handle_irq(int irq, void *dev_id)
 {
@@ -683,7 +655,7 @@ static irqreturn_t sprd_handle_irq(int irq, void *dev_id)
 		sprd_rx(port);
 
 	if (ims & SPRD_IMSR_TX_FIFO_EMPTY)
-		sprd_tx(port);
+		uart_port_tx_limit(port, THLD_TX_EMPTY);
 
 	spin_unlock(&port->lock);
 
@@ -948,6 +920,7 @@ static void sprd_poll_put_char(struct uart_port *port, unsigned char ch)
 
 static const struct uart_ops serial_sprd_ops = {
 	.tx_empty = sprd_tx_empty,
+	.put_char = sprd_put_char,
 	.get_mctrl = sprd_get_mctrl,
 	.set_mctrl = sprd_set_mctrl,
 	.stop_tx = sprd_stop_tx,
diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
index d7fd692286cf..e9bb977da576 100644
--- a/drivers/tty/serial/st-asc.c
+++ b/drivers/tty/serial/st-asc.c
@@ -238,50 +238,7 @@ static inline unsigned asc_hw_txroom(struct uart_port *port)
  */
 static void asc_transmit_chars(struct uart_port *port)
 {
-	struct circ_buf *xmit = &port->state->xmit;
-	int txroom;
-	unsigned char c;
-
-	txroom = asc_hw_txroom(port);
-
-	if ((txroom != 0) && port->x_char) {
-		c = port->x_char;
-		port->x_char = 0;
-		asc_out(port, ASC_TXBUF, c);
-		port->icount.tx++;
-		txroom = asc_hw_txroom(port);
-	}
-
-	if (uart_tx_stopped(port)) {
-		/*
-		 * We should try and stop the hardware here, but I
-		 * don't think the ASC has any way to do that.
-		 */
-		asc_disable_tx_interrupts(port);
-		return;
-	}
-
-	if (uart_circ_empty(xmit)) {
-		asc_disable_tx_interrupts(port);
-		return;
-	}
-
-	if (txroom == 0)
-		return;
-
-	do {
-		c = xmit->buf[xmit->tail];
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		asc_out(port, ASC_TXBUF, c);
-		port->icount.tx++;
-		txroom--;
-	} while ((txroom > 0) && (!uart_circ_empty(xmit)));
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	if (uart_circ_empty(xmit))
-		asc_disable_tx_interrupts(port);
+	uart_port_tx_limit(port, asc_hw_txroom(port));
 }
 
 static void asc_receive_chars(struct uart_port *port)
@@ -389,6 +346,11 @@ static unsigned int asc_tx_empty(struct uart_port *port)
 	return asc_txfifo_is_empty(port) ? TIOCSER_TEMT : 0;
 }
 
+static void asc_put_char(struct uart_port *port, unsigned char ch)
+{
+	asc_out(port, ASC_TXBUF, ch);
+}
+
 static void asc_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
 	struct asc_port *ascport = to_asc_port(port);
@@ -690,6 +652,7 @@ static void asc_put_poll_char(struct uart_port *port, unsigned char c)
 
 static const struct uart_ops asc_uart_ops = {
 	.tx_empty	= asc_tx_empty,
+	.put_char	= asc_put_char,
 	.set_mctrl	= asc_set_mctrl,
 	.get_mctrl	= asc_get_mctrl,
 	.start_tx	= asc_start_tx,
diff --git a/drivers/tty/serial/vr41xx_siu.c b/drivers/tty/serial/vr41xx_siu.c
index e0bf003ca3a1..83c28288dfe1 100644
--- a/drivers/tty/serial/vr41xx_siu.c
+++ b/drivers/tty/serial/vr41xx_siu.c
@@ -183,6 +183,11 @@ static unsigned int siu_tx_empty(struct uart_port *port)
 	return 0;
 }
 
+static void siu_put_char(struct uart_port *port, unsigned char ch)
+{
+	siu_write(port, UART_TX, ch);
+}
+
 static void siu_set_mctrl(struct uart_port *port, unsigned int mctrl)
 {
 	uint8_t mcr = 0;
@@ -370,40 +375,6 @@ static inline void check_modem_status(struct uart_port *port)
 	wake_up_interruptible(&port->state->port.delta_msr_wait);
 }
 
-static inline void transmit_chars(struct uart_port *port)
-{
-	struct circ_buf *xmit;
-	int max_count = TX_MAX_COUNT;
-
-	xmit = &port->state->xmit;
-
-	if (port->x_char) {
-		siu_write(port, UART_TX, port->x_char);
-		port->icount.tx++;
-		port->x_char = 0;
-		return;
-	}
-
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
-		siu_stop_tx(port);
-		return;
-	}
-
-	do {
-		siu_write(port, UART_TX, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	} while (max_count-- > 0);
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	if (uart_circ_empty(xmit))
-		siu_stop_tx(port);
-}
-
 static irqreturn_t siu_interrupt(int irq, void *dev_id)
 {
 	struct uart_port *port;
@@ -422,7 +393,7 @@ static irqreturn_t siu_interrupt(int irq, void *dev_id)
 	check_modem_status(port);
 
 	if (lsr & UART_LSR_THRE)
-		transmit_chars(port);
+		uart_port_tx_limit(port, TX_MAX_COUNT);
 
 	return IRQ_HANDLED;
 }
@@ -653,6 +624,7 @@ static int siu_verify_port(struct uart_port *port, struct serial_struct *serial)
 
 static const struct uart_ops siu_uart_ops = {
 	.tx_empty	= siu_tx_empty,
+	.put_char	= siu_put_char,
 	.set_mctrl	= siu_set_mctrl,
 	.get_mctrl	= siu_get_mctrl,
 	.stop_tx	= siu_stop_tx,
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] tty: serial: use uart_port_tx_limit() helper
  2022-04-11 10:54 ` [PATCH 3/3] tty: serial: use uart_port_tx_limit() helper Jiri Slaby
@ 2022-04-11 11:51   ` Pali Rohár
  2022-08-29  7:27     ` Jiri Slaby
  0 siblings, 1 reply; 10+ messages in thread
From: Pali Rohár @ 2022-04-11 11:51 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: gregkh, linux-serial, linux-kernel, Russell King

Hello!

On Monday 11 April 2022 12:54:05 Jiri Slaby wrote:
> uart_port_tx_limit() is a new helper to send characters to the device.
> Use it in these drivers.
> 
> It means we have to define two new uart hooks: tx_ready() and put_char()
> to do the real job now.
> 
> And mux.c also needs to define tx_done(). But I'm not sure if the driver
> really wants to wait for all the characters to dismiss from the HW fifo
> at this code point. Hence I marked this as FIXME.
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: bcm-kernel-feedback-list@broadcom.com
> Cc: "Pali Rohár" <pali@kernel.org>
> Cc: Kevin Cernekee <cernekee@gmail.com>
> Cc: Palmer Dabbelt <palmer@dabbelt.com>
> Cc: Paul Walmsley <paul.walmsley@sifive.com>
> Cc: Orson Zhai <orsonzhai@gmail.com>
> Cc: Baolin Wang <baolin.wang7@gmail.com>
> Cc: Chunyan Zhang <zhang.lyra@gmail.com>
> Cc: Patrice Chotard <patrice.chotard@foss.st.com>
> Cc: linux-riscv@lists.infradead.org
> ---
>  drivers/tty/serial/21285.c           | 40 +++++++--------------
>  drivers/tty/serial/altera_jtaguart.c | 43 ++++++----------------
>  drivers/tty/serial/amba-pl010.c      | 40 ++++-----------------
>  drivers/tty/serial/apbuart.c         | 37 ++++---------------
>  drivers/tty/serial/bcm63xx_uart.c    | 48 ++++++-------------------
>  drivers/tty/serial/mux.c             | 48 ++++++++-----------------
>  drivers/tty/serial/mvebu-uart.c      | 47 +++++++-----------------
>  drivers/tty/serial/omap-serial.c     | 53 +++++++---------------------
>  drivers/tty/serial/pxa.c             | 43 +++++-----------------
>  drivers/tty/serial/rp2.c             | 36 ++++++-------------
>  drivers/tty/serial/serial_txx9.c     | 40 ++++-----------------
>  drivers/tty/serial/sifive.c          | 48 ++++---------------------
>  drivers/tty/serial/sprd_serial.c     | 41 ++++-----------------
>  drivers/tty/serial/st-asc.c          | 51 ++++----------------------
>  drivers/tty/serial/vr41xx_siu.c      | 42 ++++------------------
>  15 files changed, 143 insertions(+), 514 deletions(-)
...
> diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
> index 0429c2a54290..3d07ab9eb15e 100644
> --- a/drivers/tty/serial/mvebu-uart.c
> +++ b/drivers/tty/serial/mvebu-uart.c
> @@ -194,6 +194,16 @@ static unsigned int mvebu_uart_tx_empty(struct uart_port *port)
>  	return (st & STAT_TX_EMP) ? TIOCSER_TEMT : 0;
>  }
>  
> +static bool mvebu_uart_tx_ready(struct uart_port *port)
> +{
> +	return !(readl(port->membase + UART_STAT) & STAT_TX_FIFO_FUL);

mvebu-uart.c driver in its tx_ready function should probably use
STAT_TX_RDY macro (access to STAT_TX_RDY bit in register).

Documentation for UART1 (STD) about this bit says:

This bit is set when TRANS_HLD (our UART_TSH macro) is empty and ready
for the CPU to write the next character to be transmitted out. The TSR
can still shift out the previous character when this bit is set. This
bit is cleared when the CPU writes to TRANS_HLD.

For UART2 (EXT) there is just information: UART Tx Ready for 1 Byte
Write. UART2 (EXT) has also bit (bit 5) which indicates that CPU can
load 4 bytes, but seems that this is not used by mvebu-uart.c driver.

Macro STAT_TX_RDY() is polled also in wait_for_xmitr() function.

> +}
> +
> +static void mvebu_uart_put_char(struct uart_port *port, unsigned char ch)
> +{
> +	writel(ch, port->membase + UART_TSH(port));
> +}
> +
>  static unsigned int mvebu_uart_get_mctrl(struct uart_port *port)
>  {
>  	return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
> @@ -324,40 +334,7 @@ static void mvebu_uart_rx_chars(struct uart_port *port, unsigned int status)
>  
>  static void mvebu_uart_tx_chars(struct uart_port *port, unsigned int status)
>  {
> -	struct circ_buf *xmit = &port->state->xmit;
> -	unsigned int count;
> -	unsigned int st;
> -
> -	if (port->x_char) {
> -		writel(port->x_char, port->membase + UART_TSH(port));
> -		port->icount.tx++;
> -		port->x_char = 0;
> -		return;
> -	}
> -
> -	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
> -		mvebu_uart_stop_tx(port);
> -		return;
> -	}
> -
> -	for (count = 0; count < port->fifosize; count++) {
> -		writel(xmit->buf[xmit->tail], port->membase + UART_TSH(port));
> -		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> -		port->icount.tx++;
> -
> -		if (uart_circ_empty(xmit))
> -			break;
> -
> -		st = readl(port->membase + UART_STAT);
> -		if (st & STAT_TX_FIFO_FUL)
> -			break;

I see that driver here currently use STAT_TX_FIFO_FUL for checking if it
can do next iteration and write next character to UART_TSH.

Documentation about this bit says that ... the FIFO is full, which is
indicated by TX_FIFO_FULL. TX_READY status is set as long as the FIFO is
not full. The Tx ready status is cleared when in THR FIFO is full.

In case driver does not use 4 byte loads for UART2, TX_READY and
!TX_FIFO_FULL are probably same...

Anyway, mvebu_uart_tx_chars() is called from the mvebu_uart_tx_isr()
interrupt handler which is registered for CTRL_RX_RDY_INT interrupt
which should signal when TX_READY is set.

> -	}
> -
> -	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> -		uart_write_wakeup(port);
> -
> -	if (uart_circ_empty(xmit))
> -		mvebu_uart_stop_tx(port);
> +	uart_port_tx_limit(port, port->fifosize);
>  }
>  
>  static irqreturn_t mvebu_uart_isr(int irq, void *dev_id)
> @@ -654,6 +631,8 @@ static void mvebu_uart_put_poll_char(struct uart_port *port, unsigned char c)
>  
>  static const struct uart_ops mvebu_uart_ops = {
>  	.tx_empty	= mvebu_uart_tx_empty,
> +	.tx_ready	= mvebu_uart_tx_ready,
> +	.put_char	= mvebu_uart_put_char,
>  	.set_mctrl	= mvebu_uart_set_mctrl,
>  	.get_mctrl	= mvebu_uart_get_mctrl,
>  	.stop_tx	= mvebu_uart_stop_tx,

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] tty: serial: introduce uart_port_tx{,_limit}() helpers
  2022-04-11 10:54 ` [PATCH 1/3] tty: serial: introduce uart_port_tx{,_limit}() helpers Jiri Slaby
@ 2022-04-14 16:32   ` Greg KH
  2022-04-15  7:47     ` Jiri Slaby
  0 siblings, 1 reply; 10+ messages in thread
From: Greg KH @ 2022-04-14 16:32 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: linux-serial, linux-kernel

On Mon, Apr 11, 2022 at 12:54:03PM +0200, Jiri Slaby wrote:
> Many serial drivers do the same thing:
> * send x_char if set
> * keep sending from the xmit circular buffer until either
>   - the loop reaches the end of the xmit buffer
>   - TX is stopped
>   - HW fifo is full
> * check for pending characters and:
>   - wake up tty writers to fill for more data into xmit buffer
>   - stop TX if there is nothing in the xmit buffer
> 
> The only differences are:
> * how to write the character to the HW fifo
> * the check of the end condition:
>   - is the HW fifo full?
>   - is limit of the written characters reached?
> 
> So unify the above into two helpers:
> * uart_port_tx_limit() -- the generic one, it performs the above taking
>   into account the written characters limit
> * uart_port_tx() -- calls the above with ~0 as the limit. So it only
>   checks the HW fullness.
> 
> We need three more hooks in struct uart_ops for all this to work:
> * tx_ready() -- returns true if HW can accept more data.
> * put_char() -- write a character to the device.
> * tx_done() -- when the write loop is done, perform arbitrary action
>   before potential invocation of ops->stop_tx() happens.
> 
> NOTE1: Maybe the three hooks in uart_ops above are overkill. We can
> instead pass pointers to the three functions directly to the new helpers
> as they are not used elsewhere. Similar to uart_console_write() and its
> putchar().
> 
> NOTE2: These two new helper functions call the hooks per every character
> processed. I was unable to measure any difference, provided most time is
> spent by readb (or alike) in the hooks themselves.  First, LTO might
> help to eliminate these explicit calls (we might need NOTE1 to be
> implemented for this to be true). Second, if this turns out to be a
> problem, we can introduce a macro to build the helper in the driver's
> code instead of serial_core. That is, similar to wait_event().
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---
>  Documentation/driver-api/serial/driver.rst | 28 ++++++++++++
>  drivers/tty/serial/serial_core.c           | 53 ++++++++++++++++++++++
>  include/linux/serial_core.h                |  9 ++++
>  3 files changed, 90 insertions(+)
> 
> diff --git a/Documentation/driver-api/serial/driver.rst b/Documentation/driver-api/serial/driver.rst
> index 06ec04ba086f..7dc3791addeb 100644
> --- a/Documentation/driver-api/serial/driver.rst
> +++ b/Documentation/driver-api/serial/driver.rst
> @@ -80,6 +80,34 @@ hardware.
>  
>  	This call must not sleep
>  
> +  tx_ready(port)
> +	The driver returns true if the HW can accept more data to be sent.
> +
> +	Locking: port->lock taken.
> +
> +	Interrupts: locally disabled.
> +
> +	This call must not sleep.
> +
> +  put_char(port, ch)
> +	The driver is asked to write ch to the device.
> +
> +	Locking: port->lock taken.
> +
> +	Interrupts: locally disabled.
> +
> +	This call must not sleep.
> +
> +  tx_done(port)
> +	When the write loop is done, the driver can perform arbitrary action
> +	here before potential invocation of ops->stop_tx() happens.
> +
> +	Locking: port->lock taken.
> +
> +	Interrupts: locally disabled.
> +
> +	This call must not sleep.
> +
>    set_mctrl(port, mctrl)
>  	This function sets the modem control lines for port described
>  	by 'port' to the state described by mctrl.  The relevant bits
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 6a8963caf954..1be14e90066c 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -107,6 +107,59 @@ void uart_write_wakeup(struct uart_port *port)
>  }
>  EXPORT_SYMBOL(uart_write_wakeup);
>  
> +static bool uart_port_tx_always_ready(struct uart_port *port)
> +{
> +	return true;
> +}
> +
> +/**
> + * uart_port_tx_limit -- transmit helper for uart_port
> + * @port: from which port to transmit
> + * @count: limit count
> + *
> + * uart_port_tx_limit() transmits characters from the xmit buffer to the
> + * hardware using @uart_port::ops::put_char(). It does so until @count
> + * characters are sent and while @uart_port::ops::tx_ready() still returns
> + * non-zero (if non-NULL).
> + *
> + * Return: number of characters in the xmit buffer when done.
> + */
> +unsigned int uart_port_tx_limit(struct uart_port *port, unsigned int count)
> +{
> +	struct circ_buf *xmit = &port->state->xmit;
> +	bool (*tx_ready)(struct uart_port *) = port->ops->tx_ready ? :
> +		uart_port_tx_always_ready;
> +	unsigned int pending;
> +
> +	for (; count && tx_ready(port); count--, port->icount.tx++) {
> +		if (port->x_char) {
> +			port->ops->put_char(port, port->x_char);
> +			port->x_char = 0;
> +			continue;
> +		}
> +
> +		if (uart_circ_empty(xmit) || uart_tx_stopped(port))
> +			break;
> +
> +		port->ops->put_char(port, xmit->buf[xmit->tail]);

That's a lot of redirection and function pointer mess per each character
sent now.  With the spectre overhead here (and only getting worse), this
feels like a step backwards.

I doubt throughput matters here given cpu speeds now, _but_ the cpu load
should go up.

Although on smaller cpus with slower Mhz and faster line rates, this
feels like a lot of extra work happening for no real good reason.

Any benchmarks?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] tty: serial: introduce uart_port_tx{,_limit}() helpers
  2022-04-14 16:32   ` Greg KH
@ 2022-04-15  7:47     ` Jiri Slaby
  2022-04-15  8:42       ` Greg KH
  0 siblings, 1 reply; 10+ messages in thread
From: Jiri Slaby @ 2022-04-15  7:47 UTC (permalink / raw)
  To: Greg KH; +Cc: linux-serial, linux-kernel

On 14. 04. 22, 18:32, Greg KH wrote:
> On Mon, Apr 11, 2022 at 12:54:03PM +0200, Jiri Slaby wrote:
>> Many serial drivers do the same thing:
>> * send x_char if set
>> * keep sending from the xmit circular buffer until either
>>    - the loop reaches the end of the xmit buffer
>>    - TX is stopped
>>    - HW fifo is full
>> * check for pending characters and:
>>    - wake up tty writers to fill for more data into xmit buffer
>>    - stop TX if there is nothing in the xmit buffer
>>
>> The only differences are:
>> * how to write the character to the HW fifo
>> * the check of the end condition:
>>    - is the HW fifo full?
>>    - is limit of the written characters reached?
>>
>> So unify the above into two helpers:
>> * uart_port_tx_limit() -- the generic one, it performs the above taking
>>    into account the written characters limit
>> * uart_port_tx() -- calls the above with ~0 as the limit. So it only
>>    checks the HW fullness.
>>
>> We need three more hooks in struct uart_ops for all this to work:
>> * tx_ready() -- returns true if HW can accept more data.
>> * put_char() -- write a character to the device.
>> * tx_done() -- when the write loop is done, perform arbitrary action
>>    before potential invocation of ops->stop_tx() happens.
>>
>> NOTE1: Maybe the three hooks in uart_ops above are overkill. We can
>> instead pass pointers to the three functions directly to the new helpers
>> as they are not used elsewhere. Similar to uart_console_write() and its
>> putchar().
>>
>> NOTE2: These two new helper functions call the hooks per every character
>> processed. I was unable to measure any difference, provided most time is
>> spent by readb (or alike) in the hooks themselves.  First, LTO might
>> help to eliminate these explicit calls (we might need NOTE1 to be
>> implemented for this to be true). Second, if this turns out to be a
>> problem, we can introduce a macro to build the helper in the driver's
>> code instead of serial_core. That is, similar to wait_event().
>>
>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>> ---
>>   Documentation/driver-api/serial/driver.rst | 28 ++++++++++++
>>   drivers/tty/serial/serial_core.c           | 53 ++++++++++++++++++++++
>>   include/linux/serial_core.h                |  9 ++++
>>   3 files changed, 90 insertions(+)
>>
>> diff --git a/Documentation/driver-api/serial/driver.rst b/Documentation/driver-api/serial/driver.rst
>> index 06ec04ba086f..7dc3791addeb 100644
>> --- a/Documentation/driver-api/serial/driver.rst
>> +++ b/Documentation/driver-api/serial/driver.rst
>> @@ -80,6 +80,34 @@ hardware.
>>   
>>   	This call must not sleep
>>   
>> +  tx_ready(port)
>> +	The driver returns true if the HW can accept more data to be sent.
>> +
>> +	Locking: port->lock taken.
>> +
>> +	Interrupts: locally disabled.
>> +
>> +	This call must not sleep.
>> +
>> +  put_char(port, ch)
>> +	The driver is asked to write ch to the device.
>> +
>> +	Locking: port->lock taken.
>> +
>> +	Interrupts: locally disabled.
>> +
>> +	This call must not sleep.
>> +
>> +  tx_done(port)
>> +	When the write loop is done, the driver can perform arbitrary action
>> +	here before potential invocation of ops->stop_tx() happens.
>> +
>> +	Locking: port->lock taken.
>> +
>> +	Interrupts: locally disabled.
>> +
>> +	This call must not sleep.
>> +
>>     set_mctrl(port, mctrl)
>>   	This function sets the modem control lines for port described
>>   	by 'port' to the state described by mctrl.  The relevant bits
>> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
>> index 6a8963caf954..1be14e90066c 100644
>> --- a/drivers/tty/serial/serial_core.c
>> +++ b/drivers/tty/serial/serial_core.c
>> @@ -107,6 +107,59 @@ void uart_write_wakeup(struct uart_port *port)
>>   }
>>   EXPORT_SYMBOL(uart_write_wakeup);
>>   
>> +static bool uart_port_tx_always_ready(struct uart_port *port)
>> +{
>> +	return true;
>> +}
>> +
>> +/**
>> + * uart_port_tx_limit -- transmit helper for uart_port
>> + * @port: from which port to transmit
>> + * @count: limit count
>> + *
>> + * uart_port_tx_limit() transmits characters from the xmit buffer to the
>> + * hardware using @uart_port::ops::put_char(). It does so until @count
>> + * characters are sent and while @uart_port::ops::tx_ready() still returns
>> + * non-zero (if non-NULL).
>> + *
>> + * Return: number of characters in the xmit buffer when done.
>> + */
>> +unsigned int uart_port_tx_limit(struct uart_port *port, unsigned int count)
>> +{
>> +	struct circ_buf *xmit = &port->state->xmit;
>> +	bool (*tx_ready)(struct uart_port *) = port->ops->tx_ready ? :
>> +		uart_port_tx_always_ready;
>> +	unsigned int pending;
>> +
>> +	for (; count && tx_ready(port); count--, port->icount.tx++) {
>> +		if (port->x_char) {
>> +			port->ops->put_char(port, port->x_char);
>> +			port->x_char = 0;
>> +			continue;
>> +		}
>> +
>> +		if (uart_circ_empty(xmit) || uart_tx_stopped(port))
>> +			break;
>> +
>> +		port->ops->put_char(port, xmit->buf[xmit->tail]);
> 
> That's a lot of redirection and function pointer mess per each character
> sent now.  With the spectre overhead here (and only getting worse), this
> feels like a step backwards.
> 
> I doubt throughput matters here given cpu speeds now, _but_ the cpu load
> should go up.
> 
> Although on smaller cpus with slower Mhz and faster line rates, this
> feels like a lot of extra work happening for no real good reason.

I know… Did you miss NOTE2 in the commit log? Any idea on that?

thanks,
-- 
js
suse labs

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/3] tty: serial: introduce uart_port_tx{,_limit}() helpers
  2022-04-15  7:47     ` Jiri Slaby
@ 2022-04-15  8:42       ` Greg KH
  0 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2022-04-15  8:42 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: linux-serial, linux-kernel

On Fri, Apr 15, 2022 at 09:47:26AM +0200, Jiri Slaby wrote:
> On 14. 04. 22, 18:32, Greg KH wrote:
> > On Mon, Apr 11, 2022 at 12:54:03PM +0200, Jiri Slaby wrote:
> > > Many serial drivers do the same thing:
> > > * send x_char if set
> > > * keep sending from the xmit circular buffer until either
> > >    - the loop reaches the end of the xmit buffer
> > >    - TX is stopped
> > >    - HW fifo is full
> > > * check for pending characters and:
> > >    - wake up tty writers to fill for more data into xmit buffer
> > >    - stop TX if there is nothing in the xmit buffer
> > > 
> > > The only differences are:
> > > * how to write the character to the HW fifo
> > > * the check of the end condition:
> > >    - is the HW fifo full?
> > >    - is limit of the written characters reached?
> > > 
> > > So unify the above into two helpers:
> > > * uart_port_tx_limit() -- the generic one, it performs the above taking
> > >    into account the written characters limit
> > > * uart_port_tx() -- calls the above with ~0 as the limit. So it only
> > >    checks the HW fullness.
> > > 
> > > We need three more hooks in struct uart_ops for all this to work:
> > > * tx_ready() -- returns true if HW can accept more data.
> > > * put_char() -- write a character to the device.
> > > * tx_done() -- when the write loop is done, perform arbitrary action
> > >    before potential invocation of ops->stop_tx() happens.
> > > 
> > > NOTE1: Maybe the three hooks in uart_ops above are overkill. We can
> > > instead pass pointers to the three functions directly to the new helpers
> > > as they are not used elsewhere. Similar to uart_console_write() and its
> > > putchar().
> > > 
> > > NOTE2: These two new helper functions call the hooks per every character
> > > processed. I was unable to measure any difference, provided most time is
> > > spent by readb (or alike) in the hooks themselves.  First, LTO might
> > > help to eliminate these explicit calls (we might need NOTE1 to be
> > > implemented for this to be true). Second, if this turns out to be a
> > > problem, we can introduce a macro to build the helper in the driver's
> > > code instead of serial_core. That is, similar to wait_event().
> > > 
> > > Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> > > ---
> > >   Documentation/driver-api/serial/driver.rst | 28 ++++++++++++
> > >   drivers/tty/serial/serial_core.c           | 53 ++++++++++++++++++++++
> > >   include/linux/serial_core.h                |  9 ++++
> > >   3 files changed, 90 insertions(+)
> > > 
> > > diff --git a/Documentation/driver-api/serial/driver.rst b/Documentation/driver-api/serial/driver.rst
> > > index 06ec04ba086f..7dc3791addeb 100644
> > > --- a/Documentation/driver-api/serial/driver.rst
> > > +++ b/Documentation/driver-api/serial/driver.rst
> > > @@ -80,6 +80,34 @@ hardware.
> > >   	This call must not sleep
> > > +  tx_ready(port)
> > > +	The driver returns true if the HW can accept more data to be sent.
> > > +
> > > +	Locking: port->lock taken.
> > > +
> > > +	Interrupts: locally disabled.
> > > +
> > > +	This call must not sleep.
> > > +
> > > +  put_char(port, ch)
> > > +	The driver is asked to write ch to the device.
> > > +
> > > +	Locking: port->lock taken.
> > > +
> > > +	Interrupts: locally disabled.
> > > +
> > > +	This call must not sleep.
> > > +
> > > +  tx_done(port)
> > > +	When the write loop is done, the driver can perform arbitrary action
> > > +	here before potential invocation of ops->stop_tx() happens.
> > > +
> > > +	Locking: port->lock taken.
> > > +
> > > +	Interrupts: locally disabled.
> > > +
> > > +	This call must not sleep.
> > > +
> > >     set_mctrl(port, mctrl)
> > >   	This function sets the modem control lines for port described
> > >   	by 'port' to the state described by mctrl.  The relevant bits
> > > diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> > > index 6a8963caf954..1be14e90066c 100644
> > > --- a/drivers/tty/serial/serial_core.c
> > > +++ b/drivers/tty/serial/serial_core.c
> > > @@ -107,6 +107,59 @@ void uart_write_wakeup(struct uart_port *port)
> > >   }
> > >   EXPORT_SYMBOL(uart_write_wakeup);
> > > +static bool uart_port_tx_always_ready(struct uart_port *port)
> > > +{
> > > +	return true;
> > > +}
> > > +
> > > +/**
> > > + * uart_port_tx_limit -- transmit helper for uart_port
> > > + * @port: from which port to transmit
> > > + * @count: limit count
> > > + *
> > > + * uart_port_tx_limit() transmits characters from the xmit buffer to the
> > > + * hardware using @uart_port::ops::put_char(). It does so until @count
> > > + * characters are sent and while @uart_port::ops::tx_ready() still returns
> > > + * non-zero (if non-NULL).
> > > + *
> > > + * Return: number of characters in the xmit buffer when done.
> > > + */
> > > +unsigned int uart_port_tx_limit(struct uart_port *port, unsigned int count)
> > > +{
> > > +	struct circ_buf *xmit = &port->state->xmit;
> > > +	bool (*tx_ready)(struct uart_port *) = port->ops->tx_ready ? :
> > > +		uart_port_tx_always_ready;
> > > +	unsigned int pending;
> > > +
> > > +	for (; count && tx_ready(port); count--, port->icount.tx++) {
> > > +		if (port->x_char) {
> > > +			port->ops->put_char(port, port->x_char);
> > > +			port->x_char = 0;
> > > +			continue;
> > > +		}
> > > +
> > > +		if (uart_circ_empty(xmit) || uart_tx_stopped(port))
> > > +			break;
> > > +
> > > +		port->ops->put_char(port, xmit->buf[xmit->tail]);
> > 
> > That's a lot of redirection and function pointer mess per each character
> > sent now.  With the spectre overhead here (and only getting worse), this
> > feels like a step backwards.
> > 
> > I doubt throughput matters here given cpu speeds now, _but_ the cpu load
> > should go up.
> > 
> > Although on smaller cpus with slower Mhz and faster line rates, this
> > feels like a lot of extra work happening for no real good reason.
> 
> I know… Did you miss NOTE2 in the commit log? Any idea on that?

I did see it, but I LTO can not handle function pointer redirection.  I
was wondering if you ran any benchmarks to see if this is noticeable.

I am all for making the drivers smaller, but not at the increased
overhead of every character being sent :(

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] tty: serial: use uart_port_tx_limit() helper
  2022-04-11 11:51   ` Pali Rohár
@ 2022-08-29  7:27     ` Jiri Slaby
  2022-08-29  8:50       ` Pali Rohár
  0 siblings, 1 reply; 10+ messages in thread
From: Jiri Slaby @ 2022-08-29  7:27 UTC (permalink / raw)
  To: Pali Rohár; +Cc: gregkh, linux-serial, linux-kernel, Russell King

On 11. 04. 22, 13:51, Pali Rohár wrote:
> On Monday 11 April 2022 12:54:05 Jiri Slaby wrote:
>> uart_port_tx_limit() is a new helper to send characters to the device.
>> Use it in these drivers.
>>
>> It means we have to define two new uart hooks: tx_ready() and put_char()
>> to do the real job now.
>>
>> And mux.c also needs to define tx_done(). But I'm not sure if the driver
>> really wants to wait for all the characters to dismiss from the HW fifo
>> at this code point. Hence I marked this as FIXME.
>>
>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Cc: Florian Fainelli <f.fainelli@gmail.com>
>> Cc: bcm-kernel-feedback-list@broadcom.com
>> Cc: "Pali Rohár" <pali@kernel.org>
>> Cc: Kevin Cernekee <cernekee@gmail.com>
>> Cc: Palmer Dabbelt <palmer@dabbelt.com>
>> Cc: Paul Walmsley <paul.walmsley@sifive.com>
>> Cc: Orson Zhai <orsonzhai@gmail.com>
>> Cc: Baolin Wang <baolin.wang7@gmail.com>
>> Cc: Chunyan Zhang <zhang.lyra@gmail.com>
>> Cc: Patrice Chotard <patrice.chotard@foss.st.com>
>> Cc: linux-riscv@lists.infradead.org
>> ---
>>   drivers/tty/serial/21285.c           | 40 +++++++--------------
>>   drivers/tty/serial/altera_jtaguart.c | 43 ++++++----------------
>>   drivers/tty/serial/amba-pl010.c      | 40 ++++-----------------
>>   drivers/tty/serial/apbuart.c         | 37 ++++---------------
>>   drivers/tty/serial/bcm63xx_uart.c    | 48 ++++++-------------------
>>   drivers/tty/serial/mux.c             | 48 ++++++++-----------------
>>   drivers/tty/serial/mvebu-uart.c      | 47 +++++++-----------------
>>   drivers/tty/serial/omap-serial.c     | 53 +++++++---------------------
>>   drivers/tty/serial/pxa.c             | 43 +++++-----------------
>>   drivers/tty/serial/rp2.c             | 36 ++++++-------------
>>   drivers/tty/serial/serial_txx9.c     | 40 ++++-----------------
>>   drivers/tty/serial/sifive.c          | 48 ++++---------------------
>>   drivers/tty/serial/sprd_serial.c     | 41 ++++-----------------
>>   drivers/tty/serial/st-asc.c          | 51 ++++----------------------
>>   drivers/tty/serial/vr41xx_siu.c      | 42 ++++------------------
>>   15 files changed, 143 insertions(+), 514 deletions(-)
> ...
>> diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
>> index 0429c2a54290..3d07ab9eb15e 100644
>> --- a/drivers/tty/serial/mvebu-uart.c
>> +++ b/drivers/tty/serial/mvebu-uart.c
>> @@ -194,6 +194,16 @@ static unsigned int mvebu_uart_tx_empty(struct uart_port *port)
>>   	return (st & STAT_TX_EMP) ? TIOCSER_TEMT : 0;
>>   }
>>   
>> +static bool mvebu_uart_tx_ready(struct uart_port *port)
>> +{
>> +	return !(readl(port->membase + UART_STAT) & STAT_TX_FIFO_FUL);
> 
> mvebu-uart.c driver in its tx_ready function should probably use
> STAT_TX_RDY macro (access to STAT_TX_RDY bit in register).
> 
> Documentation for UART1 (STD) about this bit says:
> 
> This bit is set when TRANS_HLD (our UART_TSH macro) is empty and ready
> for the CPU to write the next character to be transmitted out. The TSR
> can still shift out the previous character when this bit is set. This
> bit is cleared when the CPU writes to TRANS_HLD.
> 
> For UART2 (EXT) there is just information: UART Tx Ready for 1 Byte
> Write. UART2 (EXT) has also bit (bit 5) which indicates that CPU can
> load 4 bytes, but seems that this is not used by mvebu-uart.c driver.
> 
> Macro STAT_TX_RDY() is polled also in wait_for_xmitr() function.

Hi,
so care to send fixes for the two issues :)? The series is not meant to 
change behavior...

>> +}
>> +
>> +static void mvebu_uart_put_char(struct uart_port *port, unsigned char ch)
>> +{
>> +	writel(ch, port->membase + UART_TSH(port));
>> +}
>> +
>>   static unsigned int mvebu_uart_get_mctrl(struct uart_port *port)
>>   {
>>   	return TIOCM_CTS | TIOCM_DSR | TIOCM_CAR;
>> @@ -324,40 +334,7 @@ static void mvebu_uart_rx_chars(struct uart_port *port, unsigned int status)
>>   
>>   static void mvebu_uart_tx_chars(struct uart_port *port, unsigned int status)
>>   {
>> -	struct circ_buf *xmit = &port->state->xmit;
>> -	unsigned int count;
>> -	unsigned int st;
>> -
>> -	if (port->x_char) {
>> -		writel(port->x_char, port->membase + UART_TSH(port));
>> -		port->icount.tx++;
>> -		port->x_char = 0;
>> -		return;
>> -	}
>> -
>> -	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
>> -		mvebu_uart_stop_tx(port);
>> -		return;
>> -	}
>> -
>> -	for (count = 0; count < port->fifosize; count++) {
>> -		writel(xmit->buf[xmit->tail], port->membase + UART_TSH(port));
>> -		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
>> -		port->icount.tx++;
>> -
>> -		if (uart_circ_empty(xmit))
>> -			break;
>> -
>> -		st = readl(port->membase + UART_STAT);
>> -		if (st & STAT_TX_FIFO_FUL)
>> -			break;
> 
> I see that driver here currently use STAT_TX_FIFO_FUL for checking if it
> can do next iteration and write next character to UART_TSH.
> 
> Documentation about this bit says that ... the FIFO is full, which is
> indicated by TX_FIFO_FULL. TX_READY status is set as long as the FIFO is
> not full. The Tx ready status is cleared when in THR FIFO is full.
> 
> In case driver does not use 4 byte loads for UART2, TX_READY and
> !TX_FIFO_FULL are probably same...
> 
> Anyway, mvebu_uart_tx_chars() is called from the mvebu_uart_tx_isr()
> interrupt handler which is registered for CTRL_RX_RDY_INT interrupt
> which should signal when TX_READY is set.
> 
>> -	}
>> -
>> -	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
>> -		uart_write_wakeup(port);
>> -
>> -	if (uart_circ_empty(xmit))
>> -		mvebu_uart_stop_tx(port);
>> +	uart_port_tx_limit(port, port->fifosize);
>>   }
>>   
>>   static irqreturn_t mvebu_uart_isr(int irq, void *dev_id)
>> @@ -654,6 +631,8 @@ static void mvebu_uart_put_poll_char(struct uart_port *port, unsigned char c)
>>   
>>   static const struct uart_ops mvebu_uart_ops = {
>>   	.tx_empty	= mvebu_uart_tx_empty,
>> +	.tx_ready	= mvebu_uart_tx_ready,
>> +	.put_char	= mvebu_uart_put_char,
>>   	.set_mctrl	= mvebu_uart_set_mctrl,
>>   	.get_mctrl	= mvebu_uart_get_mctrl,
>>   	.stop_tx	= mvebu_uart_stop_tx,

thanks,
-- 
js
suse labs


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 3/3] tty: serial: use uart_port_tx_limit() helper
  2022-08-29  7:27     ` Jiri Slaby
@ 2022-08-29  8:50       ` Pali Rohár
  0 siblings, 0 replies; 10+ messages in thread
From: Pali Rohár @ 2022-08-29  8:50 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: gregkh, linux-serial, linux-kernel, Russell King

On Monday 29 August 2022 09:27:01 Jiri Slaby wrote:
> On 11. 04. 22, 13:51, Pali Rohár wrote:
> > On Monday 11 April 2022 12:54:05 Jiri Slaby wrote:
> > > uart_port_tx_limit() is a new helper to send characters to the device.
> > > Use it in these drivers.
> > > 
> > > It means we have to define two new uart hooks: tx_ready() and put_char()
> > > to do the real job now.
> > > 
> > > And mux.c also needs to define tx_done(). But I'm not sure if the driver
> > > really wants to wait for all the characters to dismiss from the HW fifo
> > > at this code point. Hence I marked this as FIXME.
> > > 
> > > Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> > > Cc: Russell King <linux@armlinux.org.uk>
> > > Cc: Florian Fainelli <f.fainelli@gmail.com>
> > > Cc: bcm-kernel-feedback-list@broadcom.com
> > > Cc: "Pali Rohár" <pali@kernel.org>
> > > Cc: Kevin Cernekee <cernekee@gmail.com>
> > > Cc: Palmer Dabbelt <palmer@dabbelt.com>
> > > Cc: Paul Walmsley <paul.walmsley@sifive.com>
> > > Cc: Orson Zhai <orsonzhai@gmail.com>
> > > Cc: Baolin Wang <baolin.wang7@gmail.com>
> > > Cc: Chunyan Zhang <zhang.lyra@gmail.com>
> > > Cc: Patrice Chotard <patrice.chotard@foss.st.com>
> > > Cc: linux-riscv@lists.infradead.org
> > > ---
> > >   drivers/tty/serial/21285.c           | 40 +++++++--------------
> > >   drivers/tty/serial/altera_jtaguart.c | 43 ++++++----------------
> > >   drivers/tty/serial/amba-pl010.c      | 40 ++++-----------------
> > >   drivers/tty/serial/apbuart.c         | 37 ++++---------------
> > >   drivers/tty/serial/bcm63xx_uart.c    | 48 ++++++-------------------
> > >   drivers/tty/serial/mux.c             | 48 ++++++++-----------------
> > >   drivers/tty/serial/mvebu-uart.c      | 47 +++++++-----------------
> > >   drivers/tty/serial/omap-serial.c     | 53 +++++++---------------------
> > >   drivers/tty/serial/pxa.c             | 43 +++++-----------------
> > >   drivers/tty/serial/rp2.c             | 36 ++++++-------------
> > >   drivers/tty/serial/serial_txx9.c     | 40 ++++-----------------
> > >   drivers/tty/serial/sifive.c          | 48 ++++---------------------
> > >   drivers/tty/serial/sprd_serial.c     | 41 ++++-----------------
> > >   drivers/tty/serial/st-asc.c          | 51 ++++----------------------
> > >   drivers/tty/serial/vr41xx_siu.c      | 42 ++++------------------
> > >   15 files changed, 143 insertions(+), 514 deletions(-)
> > ...
> > > diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
> > > index 0429c2a54290..3d07ab9eb15e 100644
> > > --- a/drivers/tty/serial/mvebu-uart.c
> > > +++ b/drivers/tty/serial/mvebu-uart.c
> > > @@ -194,6 +194,16 @@ static unsigned int mvebu_uart_tx_empty(struct uart_port *port)
> > >   	return (st & STAT_TX_EMP) ? TIOCSER_TEMT : 0;
> > >   }
> > > +static bool mvebu_uart_tx_ready(struct uart_port *port)
> > > +{
> > > +	return !(readl(port->membase + UART_STAT) & STAT_TX_FIFO_FUL);
> > 
> > mvebu-uart.c driver in its tx_ready function should probably use
> > STAT_TX_RDY macro (access to STAT_TX_RDY bit in register).
> > 
> > Documentation for UART1 (STD) about this bit says:
> > 
> > This bit is set when TRANS_HLD (our UART_TSH macro) is empty and ready
> > for the CPU to write the next character to be transmitted out. The TSR
> > can still shift out the previous character when this bit is set. This
> > bit is cleared when the CPU writes to TRANS_HLD.
> > 
> > For UART2 (EXT) there is just information: UART Tx Ready for 1 Byte
> > Write. UART2 (EXT) has also bit (bit 5) which indicates that CPU can
> > load 4 bytes, but seems that this is not used by mvebu-uart.c driver.
> > 
> > Macro STAT_TX_RDY() is polled also in wait_for_xmitr() function.
> 
> Hi,
> so care to send fixes for the two issues :)? The series is not meant to
> change behavior...

Ok, I will look at it after your patches are merged to prevent merge conflicts.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-08-29  8:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-11 10:54 [PATCH 0/3] tty: TX helpers Jiri Slaby
2022-04-11 10:54 ` [PATCH 1/3] tty: serial: introduce uart_port_tx{,_limit}() helpers Jiri Slaby
2022-04-14 16:32   ` Greg KH
2022-04-15  7:47     ` Jiri Slaby
2022-04-15  8:42       ` Greg KH
2022-04-11 10:54 ` [PATCH 2/3] tty: serial: use uart_port_tx() helper Jiri Slaby
2022-04-11 10:54 ` [PATCH 3/3] tty: serial: use uart_port_tx_limit() helper Jiri Slaby
2022-04-11 11:51   ` Pali Rohár
2022-08-29  7:27     ` Jiri Slaby
2022-08-29  8:50       ` Pali Rohár

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).