All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] tty: TX helpers
@ 2022-09-01 11:06 ` Jiri Slaby
  0 siblings, 0 replies; 16+ messages in thread
From: Jiri Slaby @ 2022-09-01 11:06 UTC (permalink / raw)
  To: gregkh
  Cc: Ilpo Järvinen, 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

This series introduces DEFINE_UART_PORT_TX_HELPER +
DEFINE_UART_PORT_TX_HELPER_LIMITED TX helpers. See PATCH 1/3 for the
details. Comments welcome.

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

The diffstat of patches 2+3 is as follows:
 26 files changed, 191 insertions(+), 823 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 transmit helper generators
  tty: serial: use DEFINE_UART_PORT_TX_HELPER()
  tty: serial: use DEFINE_UART_PORT_TX_HELPER_LIMITED()

 Documentation/driver-api/serial/driver.rst |  3 +
 drivers/tty/serial/21285.c                 | 33 ++-------
 drivers/tty/serial/altera_jtaguart.c       | 42 +++--------
 drivers/tty/serial/altera_uart.c           | 37 ++--------
 drivers/tty/serial/amba-pl010.c            | 37 ++--------
 drivers/tty/serial/apbuart.c               | 36 ++-------
 drivers/tty/serial/atmel_serial.c          | 29 ++------
 drivers/tty/serial/bcm63xx_uart.c          | 47 +++---------
 drivers/tty/serial/fsl_lpuart.c            | 38 +++-------
 drivers/tty/serial/lantiq.c                | 44 +++--------
 drivers/tty/serial/lpc32xx_hs.c            | 38 ++--------
 drivers/tty/serial/mcf.c                   | 27 ++-----
 drivers/tty/serial/mpc52xx_uart.c          | 44 +----------
 drivers/tty/serial/mps2-uart.c             | 29 +-------
 drivers/tty/serial/mux.c                   | 46 ++++--------
 drivers/tty/serial/mvebu-uart.c            | 40 ++--------
 drivers/tty/serial/mxs-auart.c             | 31 ++------
 drivers/tty/serial/omap-serial.c           | 47 +++---------
 drivers/tty/serial/owl-uart.c              | 35 +--------
 drivers/tty/serial/pxa.c                   | 39 +++-------
 drivers/tty/serial/rp2.c                   | 36 ++-------
 drivers/tty/serial/sa1100.c                | 49 +++++-------
 drivers/tty/serial/serial_txx9.c           | 37 ++--------
 drivers/tty/serial/sifive.c                | 45 ++---------
 drivers/tty/serial/sprd_serial.c           | 38 ++--------
 drivers/tty/serial/st-asc.c                | 50 ++-----------
 drivers/tty/serial/vt8500_serial.c         | 40 ++--------
 include/linux/serial_core.h                | 86 ++++++++++++++++++++++
 28 files changed, 280 insertions(+), 823 deletions(-)

-- 
2.37.2


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

* [PATCH v2 0/3] tty: TX helpers
@ 2022-09-01 11:06 ` Jiri Slaby
  0 siblings, 0 replies; 16+ messages in thread
From: Jiri Slaby @ 2022-09-01 11:06 UTC (permalink / raw)
  To: gregkh
  Cc: Ilpo Järvinen, 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

This series introduces DEFINE_UART_PORT_TX_HELPER +
DEFINE_UART_PORT_TX_HELPER_LIMITED TX helpers. See PATCH 1/3 for the
details. Comments welcome.

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

The diffstat of patches 2+3 is as follows:
 26 files changed, 191 insertions(+), 823 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 transmit helper generators
  tty: serial: use DEFINE_UART_PORT_TX_HELPER()
  tty: serial: use DEFINE_UART_PORT_TX_HELPER_LIMITED()

 Documentation/driver-api/serial/driver.rst |  3 +
 drivers/tty/serial/21285.c                 | 33 ++-------
 drivers/tty/serial/altera_jtaguart.c       | 42 +++--------
 drivers/tty/serial/altera_uart.c           | 37 ++--------
 drivers/tty/serial/amba-pl010.c            | 37 ++--------
 drivers/tty/serial/apbuart.c               | 36 ++-------
 drivers/tty/serial/atmel_serial.c          | 29 ++------
 drivers/tty/serial/bcm63xx_uart.c          | 47 +++---------
 drivers/tty/serial/fsl_lpuart.c            | 38 +++-------
 drivers/tty/serial/lantiq.c                | 44 +++--------
 drivers/tty/serial/lpc32xx_hs.c            | 38 ++--------
 drivers/tty/serial/mcf.c                   | 27 ++-----
 drivers/tty/serial/mpc52xx_uart.c          | 44 +----------
 drivers/tty/serial/mps2-uart.c             | 29 +-------
 drivers/tty/serial/mux.c                   | 46 ++++--------
 drivers/tty/serial/mvebu-uart.c            | 40 ++--------
 drivers/tty/serial/mxs-auart.c             | 31 ++------
 drivers/tty/serial/omap-serial.c           | 47 +++---------
 drivers/tty/serial/owl-uart.c              | 35 +--------
 drivers/tty/serial/pxa.c                   | 39 +++-------
 drivers/tty/serial/rp2.c                   | 36 ++-------
 drivers/tty/serial/sa1100.c                | 49 +++++-------
 drivers/tty/serial/serial_txx9.c           | 37 ++--------
 drivers/tty/serial/sifive.c                | 45 ++---------
 drivers/tty/serial/sprd_serial.c           | 38 ++--------
 drivers/tty/serial/st-asc.c                | 50 ++-----------
 drivers/tty/serial/vt8500_serial.c         | 40 ++--------
 include/linux/serial_core.h                | 86 ++++++++++++++++++++++
 28 files changed, 280 insertions(+), 823 deletions(-)

-- 
2.37.2


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

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

* [PATCH v2 1/3] tty: serial: introduce transmit helper generators
  2022-09-01 11:06 ` Jiri Slaby
  (?)
@ 2022-09-01 11:06 ` Jiri Slaby
  2022-09-01 12:25   ` Greg KH
  2022-09-02 10:22   ` Ilpo Järvinen
  -1 siblings, 2 replies; 16+ messages in thread
From: Jiri Slaby @ 2022-09-01 11:06 UTC (permalink / raw)
  To: gregkh; +Cc: Ilpo Järvinen, 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 helper generators:
* DEFINE_UART_PORT_TX_HELPER_LIMITED() -- it performs the above taking
  the written characters limit into account, and
* DEFINE_UART_PORT_TX_HELPER() -- the same as above, except it only
  checks the HW readiness, not the characters limit.

The HW specific operations (as stated as "differences" above) are passed
as arguments to the macros. They are:
* 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.

Note that the above macros are generators. This means the code is
generated in place and the above 3 arguments are "inlined". I.e. no
added penalty by generating call instructions for every single
character. Nor any indirect calls. (As in previous versions of this
patchset.)

Signed-off-by: Jiri Slaby <jslaby@suse.cz>
---

Notes:
    [v2] instead of a function (uart_port_tx_limit()) in serial_core,
         generate these in-place using macros. Thus eliminating "call"
         penalty.

 Documentation/driver-api/serial/driver.rst |  3 +
 include/linux/serial_core.h                | 86 ++++++++++++++++++++++
 2 files changed, 89 insertions(+)

diff --git a/Documentation/driver-api/serial/driver.rst b/Documentation/driver-api/serial/driver.rst
index 23c6b956cd90..25775bf1fcc6 100644
--- a/Documentation/driver-api/serial/driver.rst
+++ b/Documentation/driver-api/serial/driver.rst
@@ -78,6 +78,9 @@ Other functions
            uart_get_lsr_info uart_handle_dcd_change uart_handle_cts_change
            uart_try_toggle_sysrq uart_get_console
 
+.. kernel-doc:: include/linux/serial_core.h
+   :identifiers: DEFINE_UART_PORT_TX_HELPER_LIMITED DEFINE_UART_PORT_TX_HELPER
+
 Other notes
 -----------
 
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index 6e4f4765d209..715778160ae1 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -646,6 +646,92 @@ struct uart_driver {
 
 void uart_write_wakeup(struct uart_port *port);
 
+#define __DEFINE_UART_PORT_TX_HELPER(name, port, ch, tx_ready, put_char,  \
+		tx_done, for_test, for_post, ...)			  \
+unsigned int name(struct uart_port *port __VA_OPT__(,) __VA_ARGS__)	  \
+{									  \
+	struct circ_buf *xmit = &port->state->xmit;			  \
+	unsigned int pending;						  \
+	u8 ch;								  \
+									  \
+	for (; (for_test) && (tx_ready); (for_post), port->icount.tx++) { \
+		if (port->x_char) {					  \
+			ch = port->x_char;				  \
+			(put_char);					  \
+			port->x_char = 0;				  \
+			continue;					  \
+		}							  \
+									  \
+		if (uart_circ_empty(xmit) || uart_tx_stopped(port))	  \
+			break;						  \
+									  \
+		ch = xmit->buf[xmit->tail];				  \
+		(put_char);						  \
+		xmit->tail = (xmit->tail + 1) % UART_XMIT_SIZE;		  \
+	}								  \
+									  \
+	(tx_done);							  \
+									  \
+	pending = uart_circ_chars_pending(xmit);			  \
+	if (pending < WAKEUP_CHARS) {					  \
+		uart_write_wakeup(port);				  \
+									  \
+		if (pending == 0)					  \
+			port->ops->stop_tx(port);			  \
+	}								  \
+									  \
+	return pending;							  \
+}
+
+/**
+ * DEFINE_UART_PORT_TX_HELPER_LIMITED -- generate transmit helper for uart_port
+ *	with count limiting
+ * @name: name of the helper to generate
+ * @port: name of variable holding uart_port
+ * @ch: name of variable holding a character to write
+ * @tx_ready: can HW accept more data function
+ * @put_char: function to write a character
+ * @tx_done: function to call after the loop is done
+ *
+ * This macro generates a function @name. The generated function is meant as a
+ * helper to transmit characters from the xmit buffer to the hardware using
+ * @put_char(). It does so until count (passed to @name) characters are sent
+ * and while @tx_ready() still returns non-zero (if non-NULL).
+ *
+ * The generated function returns the number of characters in the xmit buffer
+ * when done.
+ *
+ * The functions in parameters shall be designed as follows:
+ *  * **tx_ready(port):** the function shall return true if the HW can accept
+ *    more data to be sent. This function can be %NULL, which means the HW is
+ *    always ready.
+ *  * **put_char(port, ch):** the function shall write @ch to the device.
+ *  * **tx_done(port):** when the write loop is done, this function can
+ *    perform arbitrary action before potential invocation of ops->stop_tx()
+ *    happens. This function can be %NULL.
+ *
+ * For all of them, @port->lock is held, interrupts are locally disabled and
+ * the functions must not sleep.
+ */
+#define DEFINE_UART_PORT_TX_HELPER_LIMITED(name, port, ch, tx_ready, put_char, \
+		                tx_done)				       \
+	__DEFINE_UART_PORT_TX_HELPER(name, port, ch, tx_ready, put_char,       \
+			tx_done, count, count--, unsigned int count)
+
+/**
+ * DEFINE_UART_PORT_TX_HELPER -- generate transmit helper for uart_port
+ * @name: name of the helper to generate
+ * @port: name of variable holding uart_port
+ * @ch: name of variable holding a character to write
+ * @tx_ready: can HW accept more data function
+ * @put_char: function to write a character
+ *
+ * See DEFINE_UART_PORT_TX_HELPER_LIMITED() for more details.
+ */
+#define DEFINE_UART_PORT_TX_HELPER(name, port, ch, tx_ready, put_char)	 \
+	__DEFINE_UART_PORT_TX_HELPER(name, port, ch, tx_ready, put_char, \
+			({}), true, ({}))
+
 /*
  * Baud rate helpers.
  */
-- 
2.37.2


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

* [PATCH v2 2/3] tty: serial: use DEFINE_UART_PORT_TX_HELPER()
  2022-09-01 11:06 ` Jiri Slaby
  (?)
  (?)
@ 2022-09-01 11:06 ` Jiri Slaby
  2022-09-02 14:21   ` Ilpo Järvinen
  -1 siblings, 1 reply; 16+ messages in thread
From: Jiri Slaby @ 2022-09-01 11:06 UTC (permalink / raw)
  To: gregkh
  Cc: Ilpo Järvinen, 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

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

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   | 37 +++-------------------
 drivers/tty/serial/atmel_serial.c  | 29 +++++-------------
 drivers/tty/serial/fsl_lpuart.c    | 38 ++++++-----------------
 drivers/tty/serial/lantiq.c        | 44 ++++++++-------------------
 drivers/tty/serial/lpc32xx_hs.c    | 38 +++++------------------
 drivers/tty/serial/mcf.c           | 27 +++-------------
 drivers/tty/serial/mpc52xx_uart.c  | 44 ++-------------------------
 drivers/tty/serial/mps2-uart.c     | 29 ++----------------
 drivers/tty/serial/mxs-auart.c     | 31 +++++--------------
 drivers/tty/serial/owl-uart.c      | 35 ++-------------------
 drivers/tty/serial/sa1100.c        | 49 ++++++++++++------------------
 drivers/tty/serial/vt8500_serial.c | 40 +++++-------------------
 12 files changed, 88 insertions(+), 353 deletions(-)

diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index 8b749ed557c6..c85300baeb93 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -246,37 +246,10 @@ 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 DEFINE_UART_PORT_TX_HELPER(altera_uart_tx_chars, port, ch,
+		altera_uart_readl(port, ALTERA_UART_STATUS_REG) &
+		                ALTERA_UART_STATUS_TRDY_MSK,
+		altera_uart_writel(port, ch, ALTERA_UART_TXDATA_REG));
 
 static irqreturn_t altera_uart_interrupt(int irq, void *data)
 {
@@ -290,7 +263,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);
+		altera_uart_tx_chars(&pp->port);
 	spin_unlock(&port->lock);
 
 	return IRQ_RETVAL(isr);
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 30ba9eef7b39..ce0da7aa55bd 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -812,36 +812,21 @@ static void atmel_rx_chars(struct uart_port *port)
 	atmel_tasklet_schedule(atmel_port, &atmel_port->tasklet_rx);
 }
 
+static DEFINE_UART_PORT_TX_HELPER(atmel_do_tx_chars, port, ch,
+		atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY,
+		atmel_uart_write_char(port, ch));
+
 /*
  * Transmit characters (called from tasklet with TXRDY interrupt
  * disabled)
  */
 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 = atmel_do_tx_chars(port);
+	if (pending) {
 		/* we still have characters to transmit, so we should continue
 		 * transmitting them when TX is ready, regardless of
 		 * mode or duplexity
diff --git a/drivers/tty/serial/fsl_lpuart.c b/drivers/tty/serial/fsl_lpuart.c
index f6c33cd228c8..7a28c0e37142 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -740,36 +740,18 @@ static int lpuart32_poll_get_char(struct uart_port *port)
 }
 #endif
 
-static inline void lpuart_transmit_buffer(struct lpuart_port *sport)
+static bool lpuart_tx_ready(struct uart_port *port)
 {
-	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++;
-	}
+	struct lpuart_port *sport = container_of(port, struct lpuart_port,
+			port);
 
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(&sport->port);
-
-	if (uart_circ_empty(xmit))
-		lpuart_stop_tx(&sport->port);
+	return readb(port->membase + UARTTCFIFO) < sport->txfifo_size;
 }
 
+static DEFINE_UART_PORT_TX_HELPER(lpuart_transmit_buffer, port, ch,
+		lpuart_tx_ready(port),
+		writeb(ch, port->membase + UARTDR));
+
 static inline void lpuart32_transmit_buffer(struct lpuart_port *sport)
 {
 	struct circ_buf *xmit = &sport->port.state->xmit;
@@ -820,7 +802,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);
+			lpuart_transmit_buffer(&sport->port);
 	}
 }
 
@@ -877,7 +859,7 @@ static unsigned int lpuart32_tx_empty(struct uart_port *port)
 static void lpuart_txint(struct lpuart_port *sport)
 {
 	spin_lock(&sport->port.lock);
-	lpuart_transmit_buffer(sport);
+	lpuart_transmit_buffer(&sport->port);
 	spin_unlock(&sport->port.lock);
 }
 
diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
index a3120c3347dd..d4b6ad594d8f 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;
 
@@ -139,6 +138,18 @@ lqasc_stop_tx(struct uart_port *port)
 	return;
 }
 
+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 DEFINE_UART_PORT_TX_HELPER(lqasc_tx_chars, port, ch,
+		lqasc_tx_ready(port),
+		writeb(ch, port->membase + LTQ_ASC_TBUF));
+
 static void
 lqasc_start_tx(struct uart_port *port)
 {
@@ -219,37 +230,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)
 {
diff --git a/drivers/tty/serial/lpc32xx_hs.c b/drivers/tty/serial/lpc32xx_hs.c
index 93140cac1ca1..07421b8433c8 100644
--- a/drivers/tty/serial/lpc32xx_hs.c
+++ b/drivers/tty/serial/lpc32xx_hs.c
@@ -276,41 +276,17 @@ 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)
+static bool serial_lpc32xx_tx_ready(struct uart_port *port)
 {
-	struct circ_buf *xmit = &port->state->xmit;
+	u32 level = readl(LPC32XX_HSUART_LEVEL(port->membase));
 
-	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);
+	return LPC32XX_HSU_TX_LEV(level) < 64;
 }
 
+static DEFINE_UART_PORT_TX_HELPER(__serial_lpc32xx_tx, port, ch,
+		serial_lpc32xx_tx_ready(port),
+		writel(ch, LPC32XX_HSUART_FIFO(port->membase)));
+
 static irqreturn_t serial_lpc32xx_interrupt(int irq, void *dev_id)
 {
 	struct uart_port *port = dev_id;
diff --git a/drivers/tty/serial/mcf.c b/drivers/tty/serial/mcf.c
index f4aaaadd0742..32a86d275c35 100644
--- a/drivers/tty/serial/mcf.c
+++ b/drivers/tty/serial/mcf.c
@@ -324,32 +324,15 @@ static void mcf_rx_chars(struct mcf_uart *pp)
 
 /****************************************************************************/
 
+static DEFINE_UART_PORT_TX_HELPER(mcf_do_tx_chars, port, ch,
+		readb(port->membase + MCFUART_USR) & MCFUART_USR_TXREADY,
+		writeb(ch, port->membase + MCFUART_UTB));
+
 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 (!mcf_do_tx_chars(port)) {
 		/* Disable TX to negate RTS automatically */
 		if (port->rs485.flags & SER_RS485_ENABLED)
 			writeb(MCFUART_UCR_TXDISABLE,
diff --git a/drivers/tty/serial/mpc52xx_uart.c b/drivers/tty/serial/mpc52xx_uart.c
index 3f1986c89694..944686a0d00d 100644
--- a/drivers/tty/serial/mpc52xx_uart.c
+++ b/drivers/tty/serial/mpc52xx_uart.c
@@ -1338,7 +1338,6 @@ 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,
 	.set_mctrl	= mpc52xx_uart_set_mctrl,
@@ -1425,46 +1424,9 @@ mpc52xx_uart_int_rx_chars(struct uart_port *port)
 	return psc_ops->raw_rx_rdy(port);
 }
 
-static inline 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;
-}
+static DEFINE_UART_PORT_TX_HELPER(mpc52xx_uart_int_tx_chars, port, ch,
+		psc_ops->raw_tx_rdy(port),
+		psc_ops->write_char(port, ch));
 
 static irqreturn_t
 mpc5xxx_uart_process_int(struct uart_port *port)
diff --git a/drivers/tty/serial/mps2-uart.c b/drivers/tty/serial/mps2-uart.c
index 5e9429dcc51f..e93c0bf7fa88 100644
--- a/drivers/tty/serial/mps2-uart.c
+++ b/drivers/tty/serial/mps2-uart.c
@@ -127,32 +127,9 @@ 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 DEFINE_UART_PORT_TX_HELPER(mps2_uart_tx_chars,  port, ch,
+		mps2_uart_tx_empty(port),
+		mps2_uart_write8(port, ch, UARTn_DATA));
 
 static void mps2_uart_start_tx(struct uart_port *port)
 {
diff --git a/drivers/tty/serial/mxs-auart.c b/drivers/tty/serial/mxs-auart.c
index 1944daf8593a..4fb04b7b9b88 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -566,6 +566,10 @@ static int mxs_auart_dma_tx(struct mxs_auart_port *s, int size)
 	return 0;
 }
 
+static DEFINE_UART_PORT_TX_HELPER(mxs_auart_do_tx_chars, port, ch,
+		!(mxs_read(to_auart_port(port), REG_STAT) & AUART_STAT_TXFF),
+		mxs_write(ch, to_auart_port(port), REG_DATA));
+
 static void mxs_auart_tx_chars(struct mxs_auart_port *s)
 {
 	struct circ_buf *xmit = &s->port.state->xmit;
@@ -603,31 +607,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 (mxs_auart_do_tx_chars(&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)
diff --git a/drivers/tty/serial/owl-uart.c b/drivers/tty/serial/owl-uart.c
index 888e17e3f25f..678a8ae5711a 100644
--- a/drivers/tty/serial/owl-uart.c
+++ b/drivers/tty/serial/owl-uart.c
@@ -179,38 +179,9 @@ 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) & (UART_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 DEFINE_UART_PORT_TX_HELPER(owl_uart_send_chars, port, ch,
+		!(owl_uart_read(port, OWL_UART_STAT) & OWL_UART_STAT_TFFU),
+		owl_uart_write(port, ch, OWL_UART_TXDAT));
 
 static void owl_uart_receive_chars(struct uart_port *port)
 {
diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
index e64e42a19d1a..738f4b20cb8e 100644
--- a/drivers/tty/serial/sa1100.c
+++ b/drivers/tty/serial/sa1100.c
@@ -226,45 +226,34 @@ sa1100_rx_chars(struct sa1100_port *sport)
 	tty_flip_buffer_push(&sport->port.state->port);
 }
 
-static void sa1100_tx_chars(struct sa1100_port *sport)
+static bool sa1100_tx_ready(struct uart_port *port)
 {
-	struct circ_buf *xmit = &sport->port.state->xmit;
+	struct sa1100_port *sport =
+		container_of(port, struct sa1100_port, port);
 
-	if (sport->port.x_char) {
-		UART_PUT_CHAR(sport, sport->port.x_char);
-		sport->port.icount.tx++;
-		sport->port.x_char = 0;
-		return;
-	}
+	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 DEFINE_UART_PORT_TX_HELPER(sa1100_do_tx_chars, port, ch,
+		sa1100_tx_ready(port), sa1100_put_char(port, ch));
+
+static void sa1100_tx_chars(struct sa1100_port *sport)
+{
 	/*
 	 * 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);
+	sa1100_do_tx_chars(&sport->port);
 }
 
 static irqreturn_t sa1100_int(int irq, void *dev_id)
diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
index 6f08136ce78a..ff430a1cc3a2 100644
--- a/drivers/tty/serial/vt8500_serial.c
+++ b/drivers/tty/serial/vt8500_serial.c
@@ -187,37 +187,17 @@ static void handle_rx(struct uart_port *port)
 	tty_flip_buffer_push(tport);
 }
 
-static void handle_tx(struct uart_port *port)
+static unsigned int vt8500_tx_empty(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;
-	}
+	unsigned int idx = vt8500_read(port, VT8500_URFIDX) & 0x1f;
 
-	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);
+	return idx < 16 ? TIOCSER_TEMT : 0;
 }
 
+static DEFINE_UART_PORT_TX_HELPER(handle_tx, port, ch,
+		vt8500_tx_empty(port),
+		writeb(ch, port->membase + VT8500_TXFIFO));
+
 static void vt8500_start_tx(struct uart_port *port)
 {
 	struct vt8500_port *vt8500_port = container_of(port,
@@ -260,12 +240,6 @@ static irqreturn_t vt8500_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static unsigned int vt8500_tx_empty(struct uart_port *port)
-{
-	return (vt8500_read(port, VT8500_URFIDX) & 0x1f) < 16 ?
-						TIOCSER_TEMT : 0;
-}
-
 static unsigned int vt8500_get_mctrl(struct uart_port *port)
 {
 	unsigned int usr;
-- 
2.37.2


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

* [PATCH v2 3/3] tty: serial: use DEFINE_UART_PORT_TX_HELPER_LIMITED()
  2022-09-01 11:06 ` Jiri Slaby
@ 2022-09-01 11:06   ` Jiri Slaby
  -1 siblings, 0 replies; 16+ messages in thread
From: Jiri Slaby @ 2022-09-01 11:06 UTC (permalink / raw)
  To: gregkh
  Cc: Ilpo Järvinen, 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

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

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           | 33 ++++--------------
 drivers/tty/serial/altera_jtaguart.c | 42 ++++++-----------------
 drivers/tty/serial/amba-pl010.c      | 37 +++-----------------
 drivers/tty/serial/apbuart.c         | 36 ++++----------------
 drivers/tty/serial/bcm63xx_uart.c    | 47 +++++---------------------
 drivers/tty/serial/mux.c             | 46 ++++++++-----------------
 drivers/tty/serial/mvebu-uart.c      | 40 ++++------------------
 drivers/tty/serial/omap-serial.c     | 47 +++++++-------------------
 drivers/tty/serial/pxa.c             | 39 +++++-----------------
 drivers/tty/serial/rp2.c             | 36 ++++----------------
 drivers/tty/serial/serial_txx9.c     | 37 +++-----------------
 drivers/tty/serial/sifive.c          | 45 +++----------------------
 drivers/tty/serial/sprd_serial.c     | 38 +++------------------
 drivers/tty/serial/st-asc.c          | 50 ++++------------------------
 14 files changed, 103 insertions(+), 470 deletions(-)

diff --git a/drivers/tty/serial/21285.c b/drivers/tty/serial/21285.c
index 7520cc02fd4d..a23401aa0847 100644
--- a/drivers/tty/serial/21285.c
+++ b/drivers/tty/serial/21285.c
@@ -151,38 +151,17 @@ static irqreturn_t serial21285_rx_chars(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(serial21285_do_tx_chars, port, ch,
+		!(*CSR_UARTFLG & 0x20),
+		*CSR_UARTDR = ch,
+		({}));
+
 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);
+	serial21285_do_tx_chars(port, 256);
 
- out:
 	return IRQ_HANDLED;
 }
 
diff --git a/drivers/tty/serial/altera_jtaguart.c b/drivers/tty/serial/altera_jtaguart.c
index cb791c5149a3..928ed419e969 100644
--- a/drivers/tty/serial/altera_jtaguart.c
+++ b/drivers/tty/serial/altera_jtaguart.c
@@ -134,42 +134,20 @@ static void altera_jtaguart_rx_chars(struct altera_jtaguart *pp)
 	tty_flip_buffer_push(&port->state->port);
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(altera_jtaguart_do_tx_chars, port, ch,
+		true,
+		writel(ch, port->membase + ALTERA_JTAGUART_DATA_REG),
+		({}));
+
 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;
+	altera_jtaguart_do_tx_chars(port, space);
 }
 
 static irqreturn_t altera_jtaguart_interrupt(int irq, void *data)
diff --git a/drivers/tty/serial/amba-pl010.c b/drivers/tty/serial/amba-pl010.c
index fae0b581ff42..cc95e5546b79 100644
--- a/drivers/tty/serial/amba-pl010.c
+++ b/drivers/tty/serial/amba-pl010.c
@@ -162,37 +162,10 @@ 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 DEFINE_UART_PORT_TX_HELPER_LIMITED(pl010_tx_chars, port, ch,
+		true,
+		writel(ch, port->membase + UART01x_DR),
+		({}));
 
 static void pl010_modem_status(struct uart_amba_port *uap)
 {
@@ -238,7 +211,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);
+				pl010_tx_chars(port, port->fifosize >> 1);
 
 			if (pass_counter-- == 0)
 				break;
diff --git a/drivers/tty/serial/apbuart.c b/drivers/tty/serial/apbuart.c
index 9ef82d870ff2..06c880ed7036 100644
--- a/drivers/tty/serial/apbuart.c
+++ b/drivers/tty/serial/apbuart.c
@@ -120,38 +120,14 @@ static void apbuart_rx_chars(struct uart_port *port)
 	tty_flip_buffer_push(&port->state->port);
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(apbuart_do_tx_chars, port, ch,
+		true,
+		UART_PUT_CHAR(port, ch),
+		({}));
+
 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);
+	apbuart_do_tx_chars(port, port->fifosize >> 1);
 }
 
 static irqreturn_t apbuart_int(int irq, void *dev_id)
diff --git a/drivers/tty/serial/bcm63xx_uart.c b/drivers/tty/serial/bcm63xx_uart.c
index 53b43174aa40..1b9f0cec3aa1 100644
--- a/drivers/tty/serial/bcm63xx_uart.c
+++ b/drivers/tty/serial/bcm63xx_uart.c
@@ -297,59 +297,30 @@ static void bcm_uart_do_rx(struct uart_port *port)
 	tty_flip_buffer_push(tty_port);
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(bcm_uart_tx, port, ch,
+		true,
+		bcm_uart_writel(port, ch, UART_FIFO_REG),
+		({}));
+
 /*
  * fill tx fifo with chars to send, stop when fifo is about to be full
  * or when all chars have been sent.
  */
 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 = bcm_uart_tx(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;
 }
 
 /*
diff --git a/drivers/tty/serial/mux.c b/drivers/tty/serial/mux.c
index 0ba0f4d9459d..f65f694fdecd 100644
--- a/drivers/tty/serial/mux.c
+++ b/drivers/tty/serial/mux.c
@@ -171,6 +171,18 @@ static void mux_break_ctl(struct uart_port *port, int break_state)
 {
 }
 
+static void mux_tx_done(struct uart_port *port)
+{
+	/* FIXME js: really needs to wait? */
+	while (UART_GET_FIFO_CNT(port))
+		udelay(1);
+}
+
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(mux_transmit, port, ch,
+		true,
+		UART_PUT_CHAR(port, ch),
+		mux_tx_done(port));
+
 /**
  * mux_write - Write chars to the mux fifo.
  * @port: 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);
+	mux_transmit(port, port->fifosize - UART_GET_FIFO_CNT(port));
 }
 
 /**
diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index 65eaecd10b7c..1dc0d9bcf797 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -333,42 +333,14 @@ static void mvebu_uart_rx_chars(struct uart_port *port, unsigned int status)
 	tty_flip_buffer_push(tport);
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(mvebu_uart_do_tx_chars, port, ch,
+		!(readl(port->membase + UART_STAT) & STAT_TX_FIFO_FUL),
+		writel(ch, port->membase + UART_TSH(port)),
+		({}));
+
 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);
+	mvebu_uart_do_tx_chars(port, port->fifosize);
 }
 
 static irqreturn_t mvebu_uart_isr(int irq, void *dev_id)
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 0aa666e247d5..a0ccde3b33e3 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -337,45 +337,22 @@ 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)
+static void serial_omap_put_char(struct uart_port *port, unsigned char ch)
 {
-	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);
+	struct uart_omap_port *up = to_uart_omap_port(port);
 
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(&up->port);
+	serial_out(up, UART_TX, ch);
 
-	if (uart_circ_empty(xmit))
-		serial_omap_stop_tx(&up->port);
+	if ((up->port.rs485.flags & SER_RS485_ENABLED) &&
+	    !(up->port.rs485.flags & SER_RS485_RX_DURING_TX))
+		up->rs485_tx_filter_count++;
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(transmit_chars, port, ch,
+		true,
+		serial_omap_put_char(port, ch),
+		({}));
+
 static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)
 {
 	if (!(up->ier & UART_IER_THRI)) {
@@ -573,7 +550,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);
+			transmit_chars(&up->port, up->port.fifosize / 4);
 			break;
 		case UART_IIR_RX_TIMEOUT:
 		case UART_IIR_RDI:
diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index 9309ffd87c8e..c1376e8adbf4 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -172,39 +172,18 @@ 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)
+static void serial_pxa_put_char(struct uart_port *port, u8 ch)
 {
-	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);
-
+	struct uart_pxa_port *up = (struct uart_pxa_port *)port;
 
-	if (uart_circ_empty(xmit))
-		serial_pxa_stop_tx(&up->port);
+	serial_out(up, UART_TX, ch);
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(transmit_chars, port, ch,
+		true,
+		serial_pxa_put_char(port, ch),
+		({}));
+
 static void serial_pxa_start_tx(struct uart_port *port)
 {
 	struct uart_pxa_port *up = (struct uart_pxa_port *)port;
@@ -254,7 +233,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);
+		transmit_chars(&up->port, up->port.fifosize / 2);
 	spin_unlock(&up->port.lock);
 	return IRQ_HANDLED;
 }
diff --git a/drivers/tty/serial/rp2.c b/drivers/tty/serial/rp2.c
index 6689d8add8f7..8719022b189d 100644
--- a/drivers/tty/serial/rp2.c
+++ b/drivers/tty/serial/rp2.c
@@ -426,35 +426,10 @@ static void rp2_rx_chars(struct rp2_uart_port *up)
 	tty_flip_buffer_push(port);
 }
 
-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);
-}
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(rp2_tx_chars, port, ch,
+		true,
+		writeb(ch, port_to_up(port)->base + RP2_DATA_BYTE),
+		({}));
 
 static void rp2_ch_interrupt(struct rp2_uart_port *up)
 {
@@ -472,7 +447,8 @@ static void rp2_ch_interrupt(struct rp2_uart_port *up)
 	if (status & RP2_CHAN_STAT_RXDATA_m)
 		rp2_rx_chars(up);
 	if (status & RP2_CHAN_STAT_TXEMPTY_m)
-		rp2_tx_chars(up);
+		rp2_tx_chars(&up->port,
+			FIFO_SIZE - readw(up->base + RP2_TX_FIFO_COUNT));
 	if (status & RP2_CHAN_STAT_MS_CHANGED_MASK)
 		wake_up_interruptible(&up->port.state->port.delta_msr_wait);
 
diff --git a/drivers/tty/serial/serial_txx9.c b/drivers/tty/serial/serial_txx9.c
index 228e380db080..855ad3b457f7 100644
--- a/drivers/tty/serial/serial_txx9.c
+++ b/drivers/tty/serial/serial_txx9.c
@@ -319,37 +319,10 @@ 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 DEFINE_UART_PORT_TX_HELPER_LIMITED(transmit_chars, port, ch,
+		true,
+		sio_out(port, TXX9_SITFIFO, ch),
+		({}));
 
 static irqreturn_t serial_txx9_interrupt(int irq, void *dev_id)
 {
@@ -371,7 +344,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);
+			transmit_chars(up, TXX9_SIO_TX_FIFO);
 		/* Clear TX/RX Int. Status */
 		sio_mask(up, TXX9_SIDISR,
 			 TXX9_SIDISR_TDIS | TXX9_SIDISR_RDIS |
diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
index 5c3a07546a58..78180f936bf3 100644
--- a/drivers/tty/serial/sifive.c
+++ b/drivers/tty/serial/sifive.c
@@ -277,45 +277,10 @@ 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);
-}
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(__ssp_transmit_chars, port, ch,
+		true,
+		__ssp_transmit_char(port_to_sifive_serial_port(port), ch),
+		({}));
 
 /**
  * __ssp_enable_txwm() - enable transmit watermark interrupts
@@ -553,7 +518,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);
+		__ssp_transmit_chars(&ssp->port, SIFIVE_TX_FIFO_DEPTH);
 
 	spin_unlock(&ssp->port.lock);
 
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
index 4329b9c9cbf0..e217b7ab69af 100644
--- a/drivers/tty/serial/sprd_serial.c
+++ b/drivers/tty/serial/sprd_serial.c
@@ -624,38 +624,10 @@ 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);
-}
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(sprd_tx, port, ch,
+		true,
+		serial_out(port, SPRD_TXD, ch),
+		({}));
 
 /* 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);
+		sprd_tx(port, THLD_TX_EMPTY);
 
 	spin_unlock(&port->lock);
 
diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
index cce42f4c9bc2..f5dd3fa4bf26 100644
--- a/drivers/tty/serial/st-asc.c
+++ b/drivers/tty/serial/st-asc.c
@@ -230,6 +230,11 @@ static inline unsigned asc_hw_txroom(struct uart_port *port)
 	return 0;
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(asc_do_transmit_chars, port, ch,
+		true,
+		asc_out(port, ASC_TXBUF, ch),
+		({}));
+
 /*
  * Start transmitting chars.
  * This is called from both interrupt and task level.
@@ -237,50 +242,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);
+	asc_do_transmit_chars(port, asc_hw_txroom(port));
 }
 
 static void asc_receive_chars(struct uart_port *port)
-- 
2.37.2


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

* [PATCH v2 3/3] tty: serial: use DEFINE_UART_PORT_TX_HELPER_LIMITED()
@ 2022-09-01 11:06   ` Jiri Slaby
  0 siblings, 0 replies; 16+ messages in thread
From: Jiri Slaby @ 2022-09-01 11:06 UTC (permalink / raw)
  To: gregkh
  Cc: Ilpo Järvinen, 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

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

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           | 33 ++++--------------
 drivers/tty/serial/altera_jtaguart.c | 42 ++++++-----------------
 drivers/tty/serial/amba-pl010.c      | 37 +++-----------------
 drivers/tty/serial/apbuart.c         | 36 ++++----------------
 drivers/tty/serial/bcm63xx_uart.c    | 47 +++++---------------------
 drivers/tty/serial/mux.c             | 46 ++++++++-----------------
 drivers/tty/serial/mvebu-uart.c      | 40 ++++------------------
 drivers/tty/serial/omap-serial.c     | 47 +++++++-------------------
 drivers/tty/serial/pxa.c             | 39 +++++-----------------
 drivers/tty/serial/rp2.c             | 36 ++++----------------
 drivers/tty/serial/serial_txx9.c     | 37 +++-----------------
 drivers/tty/serial/sifive.c          | 45 +++----------------------
 drivers/tty/serial/sprd_serial.c     | 38 +++------------------
 drivers/tty/serial/st-asc.c          | 50 ++++------------------------
 14 files changed, 103 insertions(+), 470 deletions(-)

diff --git a/drivers/tty/serial/21285.c b/drivers/tty/serial/21285.c
index 7520cc02fd4d..a23401aa0847 100644
--- a/drivers/tty/serial/21285.c
+++ b/drivers/tty/serial/21285.c
@@ -151,38 +151,17 @@ static irqreturn_t serial21285_rx_chars(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(serial21285_do_tx_chars, port, ch,
+		!(*CSR_UARTFLG & 0x20),
+		*CSR_UARTDR = ch,
+		({}));
+
 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);
+	serial21285_do_tx_chars(port, 256);
 
- out:
 	return IRQ_HANDLED;
 }
 
diff --git a/drivers/tty/serial/altera_jtaguart.c b/drivers/tty/serial/altera_jtaguart.c
index cb791c5149a3..928ed419e969 100644
--- a/drivers/tty/serial/altera_jtaguart.c
+++ b/drivers/tty/serial/altera_jtaguart.c
@@ -134,42 +134,20 @@ static void altera_jtaguart_rx_chars(struct altera_jtaguart *pp)
 	tty_flip_buffer_push(&port->state->port);
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(altera_jtaguart_do_tx_chars, port, ch,
+		true,
+		writel(ch, port->membase + ALTERA_JTAGUART_DATA_REG),
+		({}));
+
 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;
+	altera_jtaguart_do_tx_chars(port, space);
 }
 
 static irqreturn_t altera_jtaguart_interrupt(int irq, void *data)
diff --git a/drivers/tty/serial/amba-pl010.c b/drivers/tty/serial/amba-pl010.c
index fae0b581ff42..cc95e5546b79 100644
--- a/drivers/tty/serial/amba-pl010.c
+++ b/drivers/tty/serial/amba-pl010.c
@@ -162,37 +162,10 @@ 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 DEFINE_UART_PORT_TX_HELPER_LIMITED(pl010_tx_chars, port, ch,
+		true,
+		writel(ch, port->membase + UART01x_DR),
+		({}));
 
 static void pl010_modem_status(struct uart_amba_port *uap)
 {
@@ -238,7 +211,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);
+				pl010_tx_chars(port, port->fifosize >> 1);
 
 			if (pass_counter-- == 0)
 				break;
diff --git a/drivers/tty/serial/apbuart.c b/drivers/tty/serial/apbuart.c
index 9ef82d870ff2..06c880ed7036 100644
--- a/drivers/tty/serial/apbuart.c
+++ b/drivers/tty/serial/apbuart.c
@@ -120,38 +120,14 @@ static void apbuart_rx_chars(struct uart_port *port)
 	tty_flip_buffer_push(&port->state->port);
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(apbuart_do_tx_chars, port, ch,
+		true,
+		UART_PUT_CHAR(port, ch),
+		({}));
+
 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);
+	apbuart_do_tx_chars(port, port->fifosize >> 1);
 }
 
 static irqreturn_t apbuart_int(int irq, void *dev_id)
diff --git a/drivers/tty/serial/bcm63xx_uart.c b/drivers/tty/serial/bcm63xx_uart.c
index 53b43174aa40..1b9f0cec3aa1 100644
--- a/drivers/tty/serial/bcm63xx_uart.c
+++ b/drivers/tty/serial/bcm63xx_uart.c
@@ -297,59 +297,30 @@ static void bcm_uart_do_rx(struct uart_port *port)
 	tty_flip_buffer_push(tty_port);
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(bcm_uart_tx, port, ch,
+		true,
+		bcm_uart_writel(port, ch, UART_FIFO_REG),
+		({}));
+
 /*
  * fill tx fifo with chars to send, stop when fifo is about to be full
  * or when all chars have been sent.
  */
 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 = bcm_uart_tx(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;
 }
 
 /*
diff --git a/drivers/tty/serial/mux.c b/drivers/tty/serial/mux.c
index 0ba0f4d9459d..f65f694fdecd 100644
--- a/drivers/tty/serial/mux.c
+++ b/drivers/tty/serial/mux.c
@@ -171,6 +171,18 @@ static void mux_break_ctl(struct uart_port *port, int break_state)
 {
 }
 
+static void mux_tx_done(struct uart_port *port)
+{
+	/* FIXME js: really needs to wait? */
+	while (UART_GET_FIFO_CNT(port))
+		udelay(1);
+}
+
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(mux_transmit, port, ch,
+		true,
+		UART_PUT_CHAR(port, ch),
+		mux_tx_done(port));
+
 /**
  * mux_write - Write chars to the mux fifo.
  * @port: 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);
+	mux_transmit(port, port->fifosize - UART_GET_FIFO_CNT(port));
 }
 
 /**
diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index 65eaecd10b7c..1dc0d9bcf797 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -333,42 +333,14 @@ static void mvebu_uart_rx_chars(struct uart_port *port, unsigned int status)
 	tty_flip_buffer_push(tport);
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(mvebu_uart_do_tx_chars, port, ch,
+		!(readl(port->membase + UART_STAT) & STAT_TX_FIFO_FUL),
+		writel(ch, port->membase + UART_TSH(port)),
+		({}));
+
 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);
+	mvebu_uart_do_tx_chars(port, port->fifosize);
 }
 
 static irqreturn_t mvebu_uart_isr(int irq, void *dev_id)
diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 0aa666e247d5..a0ccde3b33e3 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -337,45 +337,22 @@ 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)
+static void serial_omap_put_char(struct uart_port *port, unsigned char ch)
 {
-	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);
+	struct uart_omap_port *up = to_uart_omap_port(port);
 
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(&up->port);
+	serial_out(up, UART_TX, ch);
 
-	if (uart_circ_empty(xmit))
-		serial_omap_stop_tx(&up->port);
+	if ((up->port.rs485.flags & SER_RS485_ENABLED) &&
+	    !(up->port.rs485.flags & SER_RS485_RX_DURING_TX))
+		up->rs485_tx_filter_count++;
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(transmit_chars, port, ch,
+		true,
+		serial_omap_put_char(port, ch),
+		({}));
+
 static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)
 {
 	if (!(up->ier & UART_IER_THRI)) {
@@ -573,7 +550,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);
+			transmit_chars(&up->port, up->port.fifosize / 4);
 			break;
 		case UART_IIR_RX_TIMEOUT:
 		case UART_IIR_RDI:
diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index 9309ffd87c8e..c1376e8adbf4 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -172,39 +172,18 @@ 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)
+static void serial_pxa_put_char(struct uart_port *port, u8 ch)
 {
-	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);
-
+	struct uart_pxa_port *up = (struct uart_pxa_port *)port;
 
-	if (uart_circ_empty(xmit))
-		serial_pxa_stop_tx(&up->port);
+	serial_out(up, UART_TX, ch);
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(transmit_chars, port, ch,
+		true,
+		serial_pxa_put_char(port, ch),
+		({}));
+
 static void serial_pxa_start_tx(struct uart_port *port)
 {
 	struct uart_pxa_port *up = (struct uart_pxa_port *)port;
@@ -254,7 +233,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);
+		transmit_chars(&up->port, up->port.fifosize / 2);
 	spin_unlock(&up->port.lock);
 	return IRQ_HANDLED;
 }
diff --git a/drivers/tty/serial/rp2.c b/drivers/tty/serial/rp2.c
index 6689d8add8f7..8719022b189d 100644
--- a/drivers/tty/serial/rp2.c
+++ b/drivers/tty/serial/rp2.c
@@ -426,35 +426,10 @@ static void rp2_rx_chars(struct rp2_uart_port *up)
 	tty_flip_buffer_push(port);
 }
 
-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);
-}
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(rp2_tx_chars, port, ch,
+		true,
+		writeb(ch, port_to_up(port)->base + RP2_DATA_BYTE),
+		({}));
 
 static void rp2_ch_interrupt(struct rp2_uart_port *up)
 {
@@ -472,7 +447,8 @@ static void rp2_ch_interrupt(struct rp2_uart_port *up)
 	if (status & RP2_CHAN_STAT_RXDATA_m)
 		rp2_rx_chars(up);
 	if (status & RP2_CHAN_STAT_TXEMPTY_m)
-		rp2_tx_chars(up);
+		rp2_tx_chars(&up->port,
+			FIFO_SIZE - readw(up->base + RP2_TX_FIFO_COUNT));
 	if (status & RP2_CHAN_STAT_MS_CHANGED_MASK)
 		wake_up_interruptible(&up->port.state->port.delta_msr_wait);
 
diff --git a/drivers/tty/serial/serial_txx9.c b/drivers/tty/serial/serial_txx9.c
index 228e380db080..855ad3b457f7 100644
--- a/drivers/tty/serial/serial_txx9.c
+++ b/drivers/tty/serial/serial_txx9.c
@@ -319,37 +319,10 @@ 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 DEFINE_UART_PORT_TX_HELPER_LIMITED(transmit_chars, port, ch,
+		true,
+		sio_out(port, TXX9_SITFIFO, ch),
+		({}));
 
 static irqreturn_t serial_txx9_interrupt(int irq, void *dev_id)
 {
@@ -371,7 +344,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);
+			transmit_chars(up, TXX9_SIO_TX_FIFO);
 		/* Clear TX/RX Int. Status */
 		sio_mask(up, TXX9_SIDISR,
 			 TXX9_SIDISR_TDIS | TXX9_SIDISR_RDIS |
diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
index 5c3a07546a58..78180f936bf3 100644
--- a/drivers/tty/serial/sifive.c
+++ b/drivers/tty/serial/sifive.c
@@ -277,45 +277,10 @@ 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);
-}
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(__ssp_transmit_chars, port, ch,
+		true,
+		__ssp_transmit_char(port_to_sifive_serial_port(port), ch),
+		({}));
 
 /**
  * __ssp_enable_txwm() - enable transmit watermark interrupts
@@ -553,7 +518,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);
+		__ssp_transmit_chars(&ssp->port, SIFIVE_TX_FIFO_DEPTH);
 
 	spin_unlock(&ssp->port.lock);
 
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
index 4329b9c9cbf0..e217b7ab69af 100644
--- a/drivers/tty/serial/sprd_serial.c
+++ b/drivers/tty/serial/sprd_serial.c
@@ -624,38 +624,10 @@ 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);
-}
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(sprd_tx, port, ch,
+		true,
+		serial_out(port, SPRD_TXD, ch),
+		({}));
 
 /* 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);
+		sprd_tx(port, THLD_TX_EMPTY);
 
 	spin_unlock(&port->lock);
 
diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
index cce42f4c9bc2..f5dd3fa4bf26 100644
--- a/drivers/tty/serial/st-asc.c
+++ b/drivers/tty/serial/st-asc.c
@@ -230,6 +230,11 @@ static inline unsigned asc_hw_txroom(struct uart_port *port)
 	return 0;
 }
 
+static DEFINE_UART_PORT_TX_HELPER_LIMITED(asc_do_transmit_chars, port, ch,
+		true,
+		asc_out(port, ASC_TXBUF, ch),
+		({}));
+
 /*
  * Start transmitting chars.
  * This is called from both interrupt and task level.
@@ -237,50 +242,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);
+	asc_do_transmit_chars(port, asc_hw_txroom(port));
 }
 
 static void asc_receive_chars(struct uart_port *port)
-- 
2.37.2


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

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

* Re: [PATCH v2 1/3] tty: serial: introduce transmit helper generators
  2022-09-01 11:06 ` [PATCH v2 1/3] tty: serial: introduce transmit helper generators Jiri Slaby
@ 2022-09-01 12:25   ` Greg KH
  2022-09-02  5:16     ` Jiri Slaby
  2022-09-02 10:22   ` Ilpo Järvinen
  1 sibling, 1 reply; 16+ messages in thread
From: Greg KH @ 2022-09-01 12:25 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: Ilpo Järvinen, linux-serial, linux-kernel

On Thu, Sep 01, 2022 at 01:06:55PM +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 helper generators:
> * DEFINE_UART_PORT_TX_HELPER_LIMITED() -- it performs the above taking
>   the written characters limit into account, and
> * DEFINE_UART_PORT_TX_HELPER() -- the same as above, except it only
>   checks the HW readiness, not the characters limit.
> 
> The HW specific operations (as stated as "differences" above) are passed
> as arguments to the macros. They are:
> * 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.
> 
> Note that the above macros are generators. This means the code is
> generated in place and the above 3 arguments are "inlined". I.e. no
> added penalty by generating call instructions for every single
> character. Nor any indirect calls. (As in previous versions of this
> patchset.)
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---
> 
> Notes:
>     [v2] instead of a function (uart_port_tx_limit()) in serial_core,
>          generate these in-place using macros. Thus eliminating "call"
>          penalty.

Much nicer, but:

> +#define __DEFINE_UART_PORT_TX_HELPER(name, port, ch, tx_ready, put_char,  \
> +		tx_done, for_test, for_post, ...)			  \

Do you really need "port" and "ch" as part of this macro?  You always
set that to the same thing in your patches, so is it really needed?

thanks,

greg k-h

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

* Re: [PATCH v2 1/3] tty: serial: introduce transmit helper generators
  2022-09-01 12:25   ` Greg KH
@ 2022-09-02  5:16     ` Jiri Slaby
  2022-09-02  5:23       ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Jiri Slaby @ 2022-09-02  5:16 UTC (permalink / raw)
  To: Greg KH; +Cc: Ilpo Järvinen, linux-serial, linux-kernel

On 01. 09. 22, 14:25, Greg KH wrote:
> Much nicer, but:
> 
>> +#define __DEFINE_UART_PORT_TX_HELPER(name, port, ch, tx_ready, put_char,  \
>> +		tx_done, for_test, for_post, ...)			  \
> 
> Do you really need "port" and "ch" as part of this macro?  You always
> set that to the same thing in your patches, so is it really needed?

Not really, just to make obvious that those are the names that can be 
used in tx_ready, put_char... I can remove it, if you prefer, of course.

thanks,
-- 
js
suse labs


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

* Re: [PATCH v2 1/3] tty: serial: introduce transmit helper generators
  2022-09-02  5:16     ` Jiri Slaby
@ 2022-09-02  5:23       ` Greg KH
  2022-09-02  8:02         ` Jiri Slaby
  0 siblings, 1 reply; 16+ messages in thread
From: Greg KH @ 2022-09-02  5:23 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: Ilpo Järvinen, linux-serial, linux-kernel

On Fri, Sep 02, 2022 at 07:16:58AM +0200, Jiri Slaby wrote:
> On 01. 09. 22, 14:25, Greg KH wrote:
> > Much nicer, but:
> > 
> > > +#define __DEFINE_UART_PORT_TX_HELPER(name, port, ch, tx_ready, put_char,  \
> > > +		tx_done, for_test, for_post, ...)			  \
> > 
> > Do you really need "port" and "ch" as part of this macro?  You always
> > set that to the same thing in your patches, so is it really needed?
> 
> Not really, just to make obvious that those are the names that can be used
> in tx_ready, put_char... I can remove it, if you prefer, of course.

I'd recommend just removing it as it's a hard macro to read as-is.  That
would make it a bit more simple as then you are just passing in the name
and the callback functions, which makes a bit more sense to me.

thanks,

greg k-h

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

* Re: [PATCH v2 1/3] tty: serial: introduce transmit helper generators
  2022-09-02  5:23       ` Greg KH
@ 2022-09-02  8:02         ` Jiri Slaby
  0 siblings, 0 replies; 16+ messages in thread
From: Jiri Slaby @ 2022-09-02  8:02 UTC (permalink / raw)
  To: Greg KH; +Cc: Ilpo Järvinen, linux-serial, linux-kernel

On 02. 09. 22, 7:23, Greg KH wrote:
> On Fri, Sep 02, 2022 at 07:16:58AM +0200, Jiri Slaby wrote:
>> On 01. 09. 22, 14:25, Greg KH wrote:
>>> Much nicer, but:
>>>
>>>> +#define __DEFINE_UART_PORT_TX_HELPER(name, port, ch, tx_ready, put_char,  \
>>>> +		tx_done, for_test, for_post, ...)			  \
>>>
>>> Do you really need "port" and "ch" as part of this macro?  You always
>>> set that to the same thing in your patches, so is it really needed?
>>
>> Not really, just to make obvious that those are the names that can be used
>> in tx_ready, put_char... I can remove it, if you prefer, of course.
> 
> I'd recommend just removing it as it's a hard macro to read as-is.  That
> would make it a bit more simple as then you are just passing in the name
> and the callback functions, which makes a bit more sense to me.

OK, I'll wait a couple of days if more comments/acks/reviews come and 
will send a v3.

thanks,
-- 
js
suse labs


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

* Re: [PATCH v2 1/3] tty: serial: introduce transmit helper generators
  2022-09-01 11:06 ` [PATCH v2 1/3] tty: serial: introduce transmit helper generators Jiri Slaby
  2022-09-01 12:25   ` Greg KH
@ 2022-09-02 10:22   ` Ilpo Järvinen
  2022-09-02 10:24     ` Jiri Slaby
  1 sibling, 1 reply; 16+ messages in thread
From: Ilpo Järvinen @ 2022-09-02 10:22 UTC (permalink / raw)
  To: Jiri Slaby; +Cc: Greg Kroah-Hartman, linux-serial, LKML

On Thu, 1 Sep 2022, 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 helper generators:
> * DEFINE_UART_PORT_TX_HELPER_LIMITED() -- it performs the above taking
>   the written characters limit into account, and
> * DEFINE_UART_PORT_TX_HELPER() -- the same as above, except it only
>   checks the HW readiness, not the characters limit.
> 
> The HW specific operations (as stated as "differences" above) are passed
> as arguments to the macros. They are:
> * 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.
> 
> Note that the above macros are generators. This means the code is
> generated in place and the above 3 arguments are "inlined". I.e. no
> added penalty by generating call instructions for every single
> character. Nor any indirect calls. (As in previous versions of this
> patchset.)
> 
> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> ---
> 
> Notes:
>     [v2] instead of a function (uart_port_tx_limit()) in serial_core,
>          generate these in-place using macros. Thus eliminating "call"
>          penalty.
> 
>  Documentation/driver-api/serial/driver.rst |  3 +
>  include/linux/serial_core.h                | 86 ++++++++++++++++++++++
>  2 files changed, 89 insertions(+)
> 
> diff --git a/Documentation/driver-api/serial/driver.rst b/Documentation/driver-api/serial/driver.rst
> index 23c6b956cd90..25775bf1fcc6 100644
> --- a/Documentation/driver-api/serial/driver.rst
> +++ b/Documentation/driver-api/serial/driver.rst
> @@ -78,6 +78,9 @@ Other functions
>             uart_get_lsr_info uart_handle_dcd_change uart_handle_cts_change
>             uart_try_toggle_sysrq uart_get_console
>  
> +.. kernel-doc:: include/linux/serial_core.h
> +   :identifiers: DEFINE_UART_PORT_TX_HELPER_LIMITED DEFINE_UART_PORT_TX_HELPER
> +
>  Other notes
>  -----------
>  
> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
> index 6e4f4765d209..715778160ae1 100644
> --- a/include/linux/serial_core.h
> +++ b/include/linux/serial_core.h
> @@ -646,6 +646,92 @@ struct uart_driver {
>  
>  void uart_write_wakeup(struct uart_port *port);
>  
> +#define __DEFINE_UART_PORT_TX_HELPER(name, port, ch, tx_ready, put_char,  \
> +		tx_done, for_test, for_post, ...)			  \
> +unsigned int name(struct uart_port *port __VA_OPT__(,) __VA_ARGS__)	  \
> +{									  \
> +	struct circ_buf *xmit = &port->state->xmit;			  \
> +	unsigned int pending;						  \
> +	u8 ch;								  \
> +									  \
> +	for (; (for_test) && (tx_ready); (for_post), port->icount.tx++) { \

> + * The functions in parameters shall be designed as follows:
> + *  * **tx_ready(port):** the function shall return true if the HW can accept
> + *    more data to be sent. This function can be %NULL, which means the HW is
> + *    always ready.

So if tx_ready can be NULL, how does that for() loop above work??

-- 
 i.


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

* Re: [PATCH v2 1/3] tty: serial: introduce transmit helper generators
  2022-09-02 10:22   ` Ilpo Järvinen
@ 2022-09-02 10:24     ` Jiri Slaby
  0 siblings, 0 replies; 16+ messages in thread
From: Jiri Slaby @ 2022-09-02 10:24 UTC (permalink / raw)
  To: Ilpo Järvinen; +Cc: Greg Kroah-Hartman, linux-serial, LKML

On 02. 09. 22, 12:22, Ilpo Järvinen wrote:
> On Thu, 1 Sep 2022, 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 helper generators:
>> * DEFINE_UART_PORT_TX_HELPER_LIMITED() -- it performs the above taking
>>    the written characters limit into account, and
>> * DEFINE_UART_PORT_TX_HELPER() -- the same as above, except it only
>>    checks the HW readiness, not the characters limit.
>>
>> The HW specific operations (as stated as "differences" above) are passed
>> as arguments to the macros. They are:
>> * 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.
>>
>> Note that the above macros are generators. This means the code is
>> generated in place and the above 3 arguments are "inlined". I.e. no
>> added penalty by generating call instructions for every single
>> character. Nor any indirect calls. (As in previous versions of this
>> patchset.)
>>
>> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
>> ---
>>
>> Notes:
>>      [v2] instead of a function (uart_port_tx_limit()) in serial_core,
>>           generate these in-place using macros. Thus eliminating "call"
>>           penalty.
>>
>>   Documentation/driver-api/serial/driver.rst |  3 +
>>   include/linux/serial_core.h                | 86 ++++++++++++++++++++++
>>   2 files changed, 89 insertions(+)
>>
>> diff --git a/Documentation/driver-api/serial/driver.rst b/Documentation/driver-api/serial/driver.rst
>> index 23c6b956cd90..25775bf1fcc6 100644
>> --- a/Documentation/driver-api/serial/driver.rst
>> +++ b/Documentation/driver-api/serial/driver.rst
>> @@ -78,6 +78,9 @@ Other functions
>>              uart_get_lsr_info uart_handle_dcd_change uart_handle_cts_change
>>              uart_try_toggle_sysrq uart_get_console
>>   
>> +.. kernel-doc:: include/linux/serial_core.h
>> +   :identifiers: DEFINE_UART_PORT_TX_HELPER_LIMITED DEFINE_UART_PORT_TX_HELPER
>> +
>>   Other notes
>>   -----------
>>   
>> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
>> index 6e4f4765d209..715778160ae1 100644
>> --- a/include/linux/serial_core.h
>> +++ b/include/linux/serial_core.h
>> @@ -646,6 +646,92 @@ struct uart_driver {
>>   
>>   void uart_write_wakeup(struct uart_port *port);
>>   
>> +#define __DEFINE_UART_PORT_TX_HELPER(name, port, ch, tx_ready, put_char,  \
>> +		tx_done, for_test, for_post, ...)			  \
>> +unsigned int name(struct uart_port *port __VA_OPT__(,) __VA_ARGS__)	  \
>> +{									  \
>> +	struct circ_buf *xmit = &port->state->xmit;			  \
>> +	unsigned int pending;						  \
>> +	u8 ch;								  \
>> +									  \
>> +	for (; (for_test) && (tx_ready); (for_post), port->icount.tx++) { \
> 
>> + * The functions in parameters shall be designed as follows:
>> + *  * **tx_ready(port):** the function shall return true if the HW can accept
>> + *    more data to be sent. This function can be %NULL, which means the HW is
>> + *    always ready.
> 
> So if tx_ready can be NULL, how does that for() loop above work??

Sorry, the docs is wrong (corresponded to v1). I fixed it locally already:

https://git.kernel.org/pub/scm/linux/kernel/git/jirislaby/linux.git/commit/?h=devel&id=989c488c69faf2987648e09358c79d75b4a3b5c7

thanks,
-- 
js
suse labs


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

* Re: [PATCH v2 2/3] tty: serial: use DEFINE_UART_PORT_TX_HELPER()
  2022-09-01 11:06 ` [PATCH v2 2/3] tty: serial: use DEFINE_UART_PORT_TX_HELPER() Jiri Slaby
@ 2022-09-02 14:21   ` Ilpo Järvinen
  2022-09-06 10:50     ` Jiri Slaby
  0 siblings, 1 reply; 16+ messages in thread
From: Ilpo Järvinen @ 2022-09-02 14:21 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Greg Kroah-Hartman, linux-serial, LKML, 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

[-- Attachment #1: Type: text/plain, Size: 4882 bytes --]

On Thu, 1 Sep 2022, Jiri Slaby wrote:

> DEFINE_UART_PORT_TX_HELPER() is a new helper to send characters to the
> device. Use it in these drivers.
> 
> 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>
> ---

> diff --git a/drivers/tty/serial/mpc52xx_uart.c b/drivers/tty/serial/mpc52xx_uart.c
> index 3f1986c89694..944686a0d00d 100644
> --- a/drivers/tty/serial/mpc52xx_uart.c
> +++ b/drivers/tty/serial/mpc52xx_uart.c
> @@ -1338,7 +1338,6 @@ 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,
>  	.set_mctrl	= mpc52xx_uart_set_mctrl,

Stray change.

> diff --git a/drivers/tty/serial/sa1100.c b/drivers/tty/serial/sa1100.c
> index e64e42a19d1a..738f4b20cb8e 100644
> --- a/drivers/tty/serial/sa1100.c
> +++ b/drivers/tty/serial/sa1100.c
> @@ -226,45 +226,34 @@ sa1100_rx_chars(struct sa1100_port *sport)
>  	tty_flip_buffer_push(&sport->port.state->port);
>  }
>  
> -static void sa1100_tx_chars(struct sa1100_port *sport)
> +static bool sa1100_tx_ready(struct uart_port *port)
>  {
> -	struct circ_buf *xmit = &sport->port.state->xmit;
> +	struct sa1100_port *sport =
> +		container_of(port, struct sa1100_port, port);
>  
> -	if (sport->port.x_char) {
> -		UART_PUT_CHAR(sport, sport->port.x_char);
> -		sport->port.icount.tx++;
> -		sport->port.x_char = 0;
> -		return;
> -	}
> +	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);
> +}

Perhaps not to add into this change itself, but I just point out these 
would be useful in addition to what is changed:
	- Get rid of UART_PUT_CHAR()
	- sa1100_console_putchar() could use sa1100_tx_ready()

> diff --git a/drivers/tty/serial/vt8500_serial.c b/drivers/tty/serial/vt8500_serial.c
> index 6f08136ce78a..ff430a1cc3a2 100644
> --- a/drivers/tty/serial/vt8500_serial.c
> +++ b/drivers/tty/serial/vt8500_serial.c
> @@ -187,37 +187,17 @@ static void handle_rx(struct uart_port *port)
>  	tty_flip_buffer_push(tport);
>  }
>  
> -static void handle_tx(struct uart_port *port)
> +static unsigned int vt8500_tx_empty(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;
> -	}
> +	unsigned int idx = vt8500_read(port, VT8500_URFIDX) & 0x1f;
>  
> -	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);
> +	return idx < 16 ? TIOCSER_TEMT : 0;
>  }

So there's just plain move and re-layouting of existing vt8500_tx_empty() 
embedded here. Why not do it in a separate change beforehand?

I discovered this the hard way, that is, I started to look why tx on 
earth "FIFO index" < 16 should be called "tx empty", if it would be a 
separate change it would have been immediately obvious that it was a 
pre-existing thing.

> +static DEFINE_UART_PORT_TX_HELPER(handle_tx, port, ch,
> +		vt8500_tx_empty(port),
> +		writeb(ch, port->membase + VT8500_TXFIFO));
> +
>  static void vt8500_start_tx(struct uart_port *port)
>  {
>  	struct vt8500_port *vt8500_port = container_of(port,
> @@ -260,12 +240,6 @@ static irqreturn_t vt8500_irq(int irq, void *dev_id)
>  	return IRQ_HANDLED;
>  }
>  
> -static unsigned int vt8500_tx_empty(struct uart_port *port)
> -{
> -	return (vt8500_read(port, VT8500_URFIDX) & 0x1f) < 16 ?
> -						TIOCSER_TEMT : 0;
> -}
> -
>  static unsigned int vt8500_get_mctrl(struct uart_port *port)
>  {
>  	unsigned int usr;
> 

-- 
 i.

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

* Re: [PATCH v2 3/3] tty: serial: use DEFINE_UART_PORT_TX_HELPER_LIMITED()
  2022-09-01 11:06   ` Jiri Slaby
@ 2022-09-02 14:56     ` Ilpo Järvinen
  -1 siblings, 0 replies; 16+ messages in thread
From: Ilpo Järvinen @ 2022-09-02 14:56 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Greg Kroah-Hartman, linux-serial, LKML, 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

[-- Attachment #1: Type: text/plain, Size: 1043 bytes --]

On Thu, 1 Sep 2022, Jiri Slaby wrote:

> DEFINE_UART_PORT_TX_HELPER_LIMITED() is a new helper to send characters
> to the device. Use it in these drivers.
> 
> 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.

Indeed, it seems odd.

This change looked good to me but I'll give my rev-by only after seeing 
the next version.

-- 
 i.


> 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

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

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

* Re: [PATCH v2 3/3] tty: serial: use DEFINE_UART_PORT_TX_HELPER_LIMITED()
@ 2022-09-02 14:56     ` Ilpo Järvinen
  0 siblings, 0 replies; 16+ messages in thread
From: Ilpo Järvinen @ 2022-09-02 14:56 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: Greg Kroah-Hartman, linux-serial, LKML, 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

[-- Attachment #1: Type: text/plain, Size: 1043 bytes --]

On Thu, 1 Sep 2022, Jiri Slaby wrote:

> DEFINE_UART_PORT_TX_HELPER_LIMITED() is a new helper to send characters
> to the device. Use it in these drivers.
> 
> 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.

Indeed, it seems odd.

This change looked good to me but I'll give my rev-by only after seeing 
the next version.

-- 
 i.


> 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

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

* Re: [PATCH v2 2/3] tty: serial: use DEFINE_UART_PORT_TX_HELPER()
  2022-09-02 14:21   ` Ilpo Järvinen
@ 2022-09-06 10:50     ` Jiri Slaby
  0 siblings, 0 replies; 16+ messages in thread
From: Jiri Slaby @ 2022-09-06 10:50 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Greg Kroah-Hartman, linux-serial, LKML, 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

On 02. 09. 22, 16:21, Ilpo Järvinen wrote:
>> --- a/drivers/tty/serial/sa1100.c
>> +++ b/drivers/tty/serial/sa1100.c
>> @@ -226,45 +226,34 @@ sa1100_rx_chars(struct sa1100_port *sport)
>>   	tty_flip_buffer_push(&sport->port.state->port);
>>   }
>>   
>> -static void sa1100_tx_chars(struct sa1100_port *sport)
>> +static bool sa1100_tx_ready(struct uart_port *port)
>>   {
>> -	struct circ_buf *xmit = &sport->port.state->xmit;
>> +	struct sa1100_port *sport =
>> +		container_of(port, struct sa1100_port, port);
>>   
>> -	if (sport->port.x_char) {
>> -		UART_PUT_CHAR(sport, sport->port.x_char);
>> -		sport->port.icount.tx++;
>> -		sport->port.x_char = 0;
>> -		return;
>> -	}
>> +	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);
>> +}
> 
> Perhaps not to add into this change itself, but I just point out these
> would be useful in addition to what is changed:
> 	- Get rid of UART_PUT_CHAR()

Right, that would make the above easier.

> 	- sa1100_console_putchar() could use sa1100_tx_ready()
That could be a nice cleanup. But looking at the history, I am not sure 
anyone cares enough about the driver (and cleaning it up :P). And I'm 
not much into it.

I've just sent v3 including your comments addressed.

thanks,
-- 
js
suse labs


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

end of thread, other threads:[~2022-09-06 10:53 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-01 11:06 [PATCH v2 0/3] tty: TX helpers Jiri Slaby
2022-09-01 11:06 ` Jiri Slaby
2022-09-01 11:06 ` [PATCH v2 1/3] tty: serial: introduce transmit helper generators Jiri Slaby
2022-09-01 12:25   ` Greg KH
2022-09-02  5:16     ` Jiri Slaby
2022-09-02  5:23       ` Greg KH
2022-09-02  8:02         ` Jiri Slaby
2022-09-02 10:22   ` Ilpo Järvinen
2022-09-02 10:24     ` Jiri Slaby
2022-09-01 11:06 ` [PATCH v2 2/3] tty: serial: use DEFINE_UART_PORT_TX_HELPER() Jiri Slaby
2022-09-02 14:21   ` Ilpo Järvinen
2022-09-06 10:50     ` Jiri Slaby
2022-09-01 11:06 ` [PATCH v2 3/3] tty: serial: use DEFINE_UART_PORT_TX_HELPER_LIMITED() Jiri Slaby
2022-09-01 11:06   ` Jiri Slaby
2022-09-02 14:56   ` Ilpo Järvinen
2022-09-02 14:56     ` Ilpo Järvinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.