[PATCH v5 0/3] tty: TX helpers

[PATCH v5 0/3] tty: TX helpers

[Jiri Slaby (SUSE)]

This series introduces uart_port_tx() + uart_port_tx_limited() TX
helpers. See PATCH 1/3 for the details.

The preparatory cleanups were already merged, so only the helpers above
and the switch: first, to uart_port_tx() in 2/3 and then
uart_port_tx_limited() in 3/3.

I know it's pre-rc1 time now. But there is no rush. Maybe
non-maintainers can take time to review now ;).

Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Johan Hovold <johan@kernel.org>
Cc: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

Jiri Slaby (SUSE) (3):
  tty: serial: introduce transmit helpers
  tty: serial: use uart_port_tx() helper
  tty: serial: use uart_port_tx_limited()

 Documentation/driver-api/serial/driver.rst |  3 +
 drivers/tty/serial/21285.c                 | 32 ++-------
 drivers/tty/serial/altera_jtaguart.c       | 36 ++--------
 drivers/tty/serial/altera_uart.c           | 29 ++------
 drivers/tty/serial/amba-pl010.c            | 32 ++-------
 drivers/tty/serial/apbuart.c               | 34 ++-------
 drivers/tty/serial/atmel_serial.c          | 28 ++------
 drivers/tty/serial/bcm63xx_uart.c          | 47 +++----------
 drivers/tty/serial/fsl_lpuart.c            | 30 ++------
 drivers/tty/serial/lantiq.c                | 36 ++--------
 drivers/tty/serial/lpc32xx_hs.c            | 33 ++-------
 drivers/tty/serial/mcf.c                   | 34 +++------
 drivers/tty/serial/mpc52xx_uart.c          | 39 ++---------
 drivers/tty/serial/mps2-uart.c             | 26 ++-----
 drivers/tty/serial/mux.c                   | 45 ++++--------
 drivers/tty/serial/mvebu-uart.c            | 38 ++--------
 drivers/tty/serial/mxs-auart.c             | 32 +++------
 drivers/tty/serial/omap-serial.c           | 32 ++-------
 drivers/tty/serial/owl-uart.c              | 32 ++-------
 drivers/tty/serial/pxa.c                   | 33 ++-------
 drivers/tty/serial/rp2.c                   | 31 ++-------
 drivers/tty/serial/sa1100.c                | 34 ++-------
 drivers/tty/serial/serial_txx9.c           | 32 ++-------
 drivers/tty/serial/sifive.c                | 31 ++-------
 drivers/tty/serial/sprd_serial.c           | 33 ++-------
 drivers/tty/serial/st-asc.c                | 48 ++-----------
 drivers/tty/serial/vt8500_serial.c         | 30 ++------
 include/linux/serial_core.h                | 80 ++++++++++++++++++++++
 28 files changed, 228 insertions(+), 742 deletions(-)

-- 
2.37.3

[PATCH v5 1/3] tty: serial: introduce transmit helpers

[Jiri Slaby (SUSE)]

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

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

So unify the above into two helpers:
* uart_port_tx_limited() -- it performs the above taking the written
  characters limit into account, and
* uart_port_tx() -- 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 are macros. 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 some previous versions of this patchset.)

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---

Notes:
    [v4] switch from helper generators to helpers similar to wait_event()
         [Arnd]

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

diff --git a/Documentation/driver-api/serial/driver.rst b/Documentation/driver-api/serial/driver.rst
index 23c6b956cd90..98d268555dcc 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: uart_port_tx_limited uart_port_tx
+
 Other notes
 -----------
 
diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h
index d657f2a42a7b..dbbc4408bb19 100644
--- a/include/linux/serial_core.h
+++ b/include/linux/serial_core.h
@@ -664,6 +664,86 @@ struct uart_driver {
 
 void uart_write_wakeup(struct uart_port *port);
 
+#define __uart_port_tx(uport, ch, tx_ready, put_char, tx_done, for_test,      \
+		for_post)						      \
+({									      \
+	struct uart_port *__port = (uport);				      \
+	struct circ_buf *xmit = &__port->state->xmit;			      \
+	unsigned int pending;						      \
+									      \
+	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);			      \
+	}								      \
+									      \
+	pending;							      \
+})
+
+/**
+ * uart_port_tx_limited -- transmit helper for uart_port with count limiting
+ * @port: uart port
+ * @ch: variable to store a character to be written to the HW
+ * @count: a limit of characters to send
+ * @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 helper transmits characters from the xmit buffer to the hardware using
+ * @put_char(). It does so until @count characters are sent and while @tx_ready
+ * evaluates to true.
+ *
+ * Returns: the number of characters in the xmit buffer when done.
+ *
+ * The expression in macro parameters shall be designed as follows:
+ *  * **tx_ready:** should evaluate to true if the HW can accept more data to
+ *    be sent. This parameter can be %true, which means the HW is always ready.
+ *  * **put_char:** shall write @ch to the device of @port.
+ *  * **tx_done:** when the write loop is done, this can perform arbitrary
+ *    action before potential invocation of ops->stop_tx() happens. If the
+ *    driver does not need to do anything, use e.g. ({}).
+ *
+ * For all of them, @port->lock is held, interrupts are locally disabled and
+ * the expressions must not sleep.
+ */
+#define uart_port_tx_limited(port, ch, count, tx_ready, put_char, tx_done) ({ \
+	unsigned int __count = (count);					      \
+	__uart_port_tx(port, ch, tx_ready, put_char, tx_done, __count,	      \
+			__count--);					      \
+})
+
+/**
+ * uart_port_tx -- transmit helper for uart_port
+ * @port: uart port
+ * @ch: variable to store a character to be written to the HW
+ * @tx_ready: can HW accept more data function
+ * @put_char: function to write a character
+ *
+ * See uart_port_tx_limited() for more details.
+ */
+#define uart_port_tx(port, ch, tx_ready, put_char)			\
+	__uart_port_tx(port, ch, tx_ready, put_char, ({}), true, ({}))
+
 /*
  * Baud rate helpers.
  */
-- 
2.37.3

[PATCH v5 2/3] tty: serial: use uart_port_tx() helper

[Jiri Slaby (SUSE)]

uart_port_tx() 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>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---

Notes:
    [v5] use bool pending in mcf_tx_chars() too [Ilpo]
    
    [v4] switch from DEFINE_UART_PORT_TX_HELPER() (helper generator) to
         uart_port_tx() (akin to wait_event())
    
    [v3] remove stray \n removal (Ilpo)

 drivers/tty/serial/altera_uart.c   | 29 ++++------------------
 drivers/tty/serial/atmel_serial.c  | 28 +++++----------------
 drivers/tty/serial/fsl_lpuart.c    | 30 ++++-------------------
 drivers/tty/serial/lantiq.c        | 36 +++------------------------
 drivers/tty/serial/lpc32xx_hs.c    | 33 +++----------------------
 drivers/tty/serial/mcf.c           | 34 ++++++--------------------
 drivers/tty/serial/mpc52xx_uart.c  | 39 +++---------------------------
 drivers/tty/serial/mps2-uart.c     | 26 +++-----------------
 drivers/tty/serial/mxs-auart.c     | 32 ++++++------------------
 drivers/tty/serial/owl-uart.c      | 32 +++---------------------
 drivers/tty/serial/sa1100.c        | 34 +++-----------------------
 drivers/tty/serial/vt8500_serial.c | 30 +++--------------------
 12 files changed, 60 insertions(+), 323 deletions(-)

diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index 82f2790de28d..316074bb23e9 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -247,31 +247,12 @@ static void altera_uart_rx_chars(struct uart_port *port)
 
 static void altera_uart_tx_chars(struct uart_port *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);
+	u8 ch;
 
-	if (uart_circ_empty(xmit))
-		altera_uart_stop_tx(port);
+	uart_port_tx(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)
diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index bd07f79a2df9..a6b4d30c5888 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -824,30 +824,14 @@ static void atmel_rx_chars(struct uart_port *port)
  */
 static void atmel_tx_chars(struct uart_port *port)
 {
-	struct circ_buf *xmit = &port->state->xmit;
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+	bool pending;
+	u8 ch;
 
-	if (port->x_char &&
-	    (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY)) {
-		atmel_uart_write_char(port, port->x_char);
-		port->icount.tx++;
-		port->x_char = 0;
-	}
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port))
-		return;
-
-	while (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY) {
-		atmel_uart_write_char(port, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	if (!uart_circ_empty(xmit)) {
+	pending = uart_port_tx(port, ch,
+		atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY,
+		atmel_uart_write_char(port, ch));
+	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 67fa113f77d4..d811eda1844e 100644
--- a/drivers/tty/serial/fsl_lpuart.c
+++ b/drivers/tty/serial/fsl_lpuart.c
@@ -742,32 +742,12 @@ static int lpuart32_poll_get_char(struct uart_port *port)
 
 static inline void lpuart_transmit_buffer(struct lpuart_port *sport)
 {
-	struct circ_buf *xmit = &sport->port.state->xmit;
-
-	if (sport->port.x_char) {
-		writeb(sport->port.x_char, sport->port.membase + UARTDR);
-		sport->port.icount.tx++;
-		sport->port.x_char = 0;
-		return;
-	}
-
-	if (lpuart_stopped_or_empty(&sport->port)) {
-		lpuart_stop_tx(&sport->port);
-		return;
-	}
-
-	while (!uart_circ_empty(xmit) &&
-		(readb(sport->port.membase + UARTTCFIFO) < sport->txfifo_size)) {
-		writeb(xmit->buf[xmit->tail], sport->port.membase + UARTDR);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		sport->port.icount.tx++;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(&sport->port);
+	struct uart_port *port = &sport->port;
+	u8 ch;
 
-	if (uart_circ_empty(xmit))
-		lpuart_stop_tx(&sport->port);
+	uart_port_tx(port, ch,
+		readb(port->membase + UARTTCFIFO) < sport->txfifo_size,
+		writeb(ch, port->membase + UARTDR));
 }
 
 static inline void lpuart32_transmit_buffer(struct lpuart_port *sport)
diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
index c892f3c7d1ab..a58e9277dfad 100644
--- a/drivers/tty/serial/lantiq.c
+++ b/drivers/tty/serial/lantiq.c
@@ -95,7 +95,6 @@
 #define ASCFSTAT_TXFFLMASK	0x3F00
 #define ASCFSTAT_TXFREEMASK	0x3F000000
 
-static void lqasc_tx_chars(struct uart_port *port);
 static struct ltq_uart_port *lqasc_port[MAXPORTS];
 static struct uart_driver lqasc_reg;
 
@@ -151,9 +150,12 @@ lqasc_start_tx(struct uart_port *port)
 {
 	unsigned long flags;
 	struct ltq_uart_port *ltq_port = to_ltq_uart_port(port);
+	u8 ch;
 
 	spin_lock_irqsave(&ltq_port->lock, flags);
-	lqasc_tx_chars(port);
+	uart_port_tx(port, ch,
+		lqasc_tx_ready(port),
+		writeb(ch, port->membase + LTQ_ASC_TBUF));
 	spin_unlock_irqrestore(&ltq_port->lock, flags);
 	return;
 }
@@ -226,36 +228,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 (lqasc_tx_ready(port)) {
-		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 ed47f4768338..b38fe4728c26 100644
--- a/drivers/tty/serial/lpc32xx_hs.c
+++ b/drivers/tty/serial/lpc32xx_hs.c
@@ -276,8 +276,6 @@ static void __serial_lpc32xx_rx(struct uart_port *port)
 	tty_flip_buffer_push(tport);
 }
 
-static void serial_lpc32xx_stop_tx(struct uart_port *port);
-
 static bool serial_lpc32xx_tx_ready(struct uart_port *port)
 {
 	u32 level = readl(LPC32XX_HSUART_LEVEL(port->membase));
@@ -287,34 +285,11 @@ static bool serial_lpc32xx_tx_ready(struct uart_port *port)
 
 static void __serial_lpc32xx_tx(struct uart_port *port)
 {
-	struct circ_buf *xmit = &port->state->xmit;
-
-	if (port->x_char) {
-		writel((u32)port->x_char, LPC32XX_HSUART_FIFO(port->membase));
-		port->icount.tx++;
-		port->x_char = 0;
-		return;
-	}
-
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port))
-		goto exit_tx;
-
-	/* Transfer data */
-	while (serial_lpc32xx_tx_ready(port)) {
-		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);
+	u8 ch;
 
-exit_tx:
-	if (uart_circ_empty(xmit))
-		serial_lpc32xx_stop_tx(port);
+	uart_port_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)
diff --git a/drivers/tty/serial/mcf.c b/drivers/tty/serial/mcf.c
index b1cd9a76dd93..3239babe12a4 100644
--- a/drivers/tty/serial/mcf.c
+++ b/drivers/tty/serial/mcf.c
@@ -327,34 +327,16 @@ static void mcf_rx_chars(struct mcf_uart *pp)
 static void mcf_tx_chars(struct mcf_uart *pp)
 {
 	struct uart_port *port = &pp->port;
-	struct circ_buf *xmit = &port->state->xmit;
-
-	if (port->x_char) {
-		/* Send special char - probably flow control */
-		writeb(port->x_char, port->membase + MCFUART_UTB);
-		port->x_char = 0;
-		port->icount.tx++;
-		return;
-	}
-
-	while (readb(port->membase + MCFUART_USR) & MCFUART_USR_TXREADY) {
-		if (uart_circ_empty(xmit))
-			break;
-		writeb(xmit->buf[xmit->tail], port->membase + MCFUART_UTB);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE -1);
-		port->icount.tx++;
-	}
+	bool pending;
+	u8 ch;
 
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
+	pending = uart_port_tx(port, ch,
+		readb(port->membase + MCFUART_USR) & MCFUART_USR_TXREADY,
+		writeb(ch, port->membase + MCFUART_UTB));
 
-	if (uart_circ_empty(xmit)) {
-		mcf_stop_tx(port);
-		/* Disable TX to negate RTS automatically */
-		if (port->rs485.flags & SER_RS485_ENABLED)
-			writeb(MCFUART_UCR_TXDISABLE,
-				port->membase + MCFUART_UCR);
-	}
+	/* Disable TX to negate RTS automatically */
+	if (!pending && (port->rs485.flags & SER_RS485_ENABLED))
+		writeb(MCFUART_UCR_TXDISABLE, port->membase + MCFUART_UCR);
 }
 
 /****************************************************************************/
diff --git a/drivers/tty/serial/mpc52xx_uart.c b/drivers/tty/serial/mpc52xx_uart.c
index 73362d4bc45d..384ca195e3d5 100644
--- a/drivers/tty/serial/mpc52xx_uart.c
+++ b/drivers/tty/serial/mpc52xx_uart.c
@@ -1428,42 +1428,11 @@ mpc52xx_uart_int_rx_chars(struct uart_port *port)
 static inline bool
 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 true;
-	}
-
-	/* Nothing to do ? */
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
-		mpc52xx_uart_stop_tx(port);
-		return false;
-	}
-
-	/* 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 false;
-	}
+	u8 ch;
 
-	return true;
+	return uart_port_tx(port, ch,
+		psc_ops->raw_tx_rdy(port),
+		psc_ops->write_char(port, ch));
 }
 
 static irqreturn_t
diff --git a/drivers/tty/serial/mps2-uart.c b/drivers/tty/serial/mps2-uart.c
index 2e3e6cf16817..860d161fa594 100644
--- a/drivers/tty/serial/mps2-uart.c
+++ b/drivers/tty/serial/mps2-uart.c
@@ -129,29 +129,11 @@ static void mps2_uart_stop_tx(struct uart_port *port)
 
 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);
+	u8 ch;
 
-	if (uart_circ_empty(xmit))
-		mps2_uart_stop_tx(port);
+	uart_port_tx(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 d21a4f3ef2fe..ef6e7bb6105c 100644
--- a/drivers/tty/serial/mxs-auart.c
+++ b/drivers/tty/serial/mxs-auart.c
@@ -569,6 +569,8 @@ static int mxs_auart_dma_tx(struct mxs_auart_port *s, int size)
 static void mxs_auart_tx_chars(struct mxs_auart_port *s)
 {
 	struct circ_buf *xmit = &s->port.state->xmit;
+	bool pending;
+	u8 ch;
 
 	if (auart_dma_enabled(s)) {
 		u32 i = 0;
@@ -603,31 +605,13 @@ 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
+	pending = uart_port_tx(&s->port, ch,
+		!(mxs_read(s, REG_STAT) & AUART_STAT_TXFF),
+		mxs_write(ch, s, REG_DATA));
+	if (pending)
 		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 fde39cc1145d..e99970a9437f 100644
--- a/drivers/tty/serial/owl-uart.c
+++ b/drivers/tty/serial/owl-uart.c
@@ -181,35 +181,11 @@ static void owl_uart_start_tx(struct uart_port *port)
 
 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;
+	u8 ch;
 
-		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);
+	uart_port_tx(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 dd9e3253cab4..55107bbc00ce 100644
--- a/drivers/tty/serial/sa1100.c
+++ b/drivers/tty/serial/sa1100.c
@@ -228,14 +228,7 @@ sa1100_rx_chars(struct sa1100_port *sport)
 
 static void sa1100_tx_chars(struct sa1100_port *sport)
 {
-	struct circ_buf *xmit = &sport->port.state->xmit;
-
-	if (sport->port.x_char) {
-		UART_PUT_CHAR(sport, sport->port.x_char);
-		sport->port.icount.tx++;
-		sport->port.x_char = 0;
-		return;
-	}
+	u8 ch;
 
 	/*
 	 * Check the modem control lines before
@@ -243,28 +236,9 @@ static void sa1100_tx_chars(struct sa1100_port *sport)
 	 */
 	sa1100_mctrl_check(sport);
 
-	if (uart_circ_empty(xmit) || uart_tx_stopped(&sport->port)) {
-		sa1100_stop_tx(&sport->port);
-		return;
-	}
-
-	/*
-	 * Tried using FIFO (not checking TNF) for fifo fill:
-	 * still had the '4 bytes repeated' problem.
-	 */
-	while (UART_GET_UTSR1(sport) & UTSR1_TNF) {
-		UART_PUT_CHAR(sport, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		sport->port.icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(&sport->port);
-
-	if (uart_circ_empty(xmit))
-		sa1100_stop_tx(&sport->port);
+	uart_port_tx(&sport->port, ch,
+			UART_GET_UTSR1(sport) & UTSR1_TNF,
+			UART_PUT_CHAR(sport, ch));
 }
 
 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 10fbdb09965f..deedb6513160 100644
--- a/drivers/tty/serial/vt8500_serial.c
+++ b/drivers/tty/serial/vt8500_serial.c
@@ -196,33 +196,11 @@ static unsigned int vt8500_tx_empty(struct uart_port *port)
 
 static void handle_tx(struct uart_port *port)
 {
-	struct circ_buf *xmit = &port->state->xmit;
+	u8 ch;
 
-	if (port->x_char) {
-		writeb(port->x_char, port->membase + VT8500_TXFIFO);
-		port->icount.tx++;
-		port->x_char = 0;
-	}
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
-		vt8500_stop_tx(port);
-		return;
-	}
-
-	while (vt8500_tx_empty(port)) {
-		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);
+	uart_port_tx(port, ch,
+		vt8500_tx_empty(port),
+		writeb(ch, port->membase + VT8500_TXFIFO));
 }
 
 static void vt8500_start_tx(struct uart_port *port)
-- 
2.37.3

[PATCH v5 3/3] tty: serial: use uart_port_tx_limited()

[Jiri Slaby (SUSE)]

uart_port_tx_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.

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
Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>
---

Notes:
    [v4] switch from DEFINE_UART_PORT_TX_HELPER_LIMITED() (helper generator)
         to uart_port_tx_limited() (akin to wait_event())

 drivers/tty/serial/21285.c           | 32 +++----------------
 drivers/tty/serial/altera_jtaguart.c | 36 ++++-----------------
 drivers/tty/serial/amba-pl010.c      | 32 +++----------------
 drivers/tty/serial/apbuart.c         | 34 +++-----------------
 drivers/tty/serial/bcm63xx_uart.c    | 47 ++++++---------------------
 drivers/tty/serial/mux.c             | 45 ++++++++------------------
 drivers/tty/serial/mvebu-uart.c      | 38 +++-------------------
 drivers/tty/serial/omap-serial.c     | 32 +++----------------
 drivers/tty/serial/pxa.c             | 33 +++----------------
 drivers/tty/serial/rp2.c             | 31 ++++--------------
 drivers/tty/serial/serial_txx9.c     | 32 +++----------------
 drivers/tty/serial/sifive.c          | 31 +++---------------
 drivers/tty/serial/sprd_serial.c     | 33 +++----------------
 drivers/tty/serial/st-asc.c          | 48 +++-------------------------
 14 files changed, 85 insertions(+), 419 deletions(-)

diff --git a/drivers/tty/serial/21285.c b/drivers/tty/serial/21285.c
index c7d34823f715..185462fd959c 100644
--- a/drivers/tty/serial/21285.c
+++ b/drivers/tty/serial/21285.c
@@ -154,35 +154,13 @@ static irqreturn_t serial21285_rx_chars(int irq, void *dev_id)
 static irqreturn_t serial21285_tx_chars(int irq, void *dev_id)
 {
 	struct uart_port *port = dev_id;
-	struct circ_buf *xmit = &port->state->xmit;
-	int count = 256;
-
-	if (port->x_char) {
-		*CSR_UARTDR = port->x_char;
-		port->icount.tx++;
-		port->x_char = 0;
-		goto out;
-	}
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
-		serial21285_stop_tx(port);
-		goto out;
-	}
-
-	do {
-		*CSR_UARTDR = xmit->buf[xmit->tail];
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	} while (--count > 0 && !(*CSR_UARTFLG & 0x20));
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
+	u8 ch;
 
-	if (uart_circ_empty(xmit))
-		serial21285_stop_tx(port);
+	uart_port_tx_limited(port, ch, 256,
+		!(*CSR_UARTFLG & 0x20),
+		*CSR_UARTDR = ch,
+		({}));
 
- out:
 	return IRQ_HANDLED;
 }
 
diff --git a/drivers/tty/serial/altera_jtaguart.c b/drivers/tty/serial/altera_jtaguart.c
index c2d154d78e54..aa49553fac58 100644
--- a/drivers/tty/serial/altera_jtaguart.c
+++ b/drivers/tty/serial/altera_jtaguart.c
@@ -146,37 +146,15 @@ static void altera_jtaguart_rx_chars(struct altera_jtaguart *pp)
 static void altera_jtaguart_tx_chars(struct altera_jtaguart *pp)
 {
 	struct uart_port *port = &pp->port;
-	struct circ_buf *xmit = &port->state->xmit;
-	unsigned int pending, count;
-
-	if (port->x_char) {
-		/* Send special char - probably flow control */
-		writel(port->x_char, port->membase + ALTERA_JTAGUART_DATA_REG);
-		port->x_char = 0;
-		port->icount.tx++;
-		return;
-	}
+	unsigned int count;
+	u8 ch;
 
-	pending = uart_circ_chars_pending(xmit);
-	if (pending > 0) {
-		count = altera_jtaguart_tx_space(port, NULL);
-		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);
-		}
-	}
+	count = altera_jtaguart_tx_space(port, NULL);
 
-	if (pending == 0)
-		altera_jtaguart_stop_tx(port);
+	uart_port_tx_limited(port, ch, count,
+		true,
+		writel(ch, port->membase + ALTERA_JTAGUART_DATA_REG),
+		({}));
 }
 
 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 af27fb8ec145..a98fae2ca422 100644
--- a/drivers/tty/serial/amba-pl010.c
+++ b/drivers/tty/serial/amba-pl010.c
@@ -164,34 +164,12 @@ static void pl010_rx_chars(struct uart_port *port)
 
 static void pl010_tx_chars(struct uart_port *port)
 {
-	struct circ_buf *xmit = &port->state->xmit;
-	int count;
+	u8 ch;
 
-	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);
+	uart_port_tx_limited(port, ch, port->fifosize >> 1,
+		true,
+		writel(ch, port->membase + UART01x_DR),
+		({}));
 }
 
 static void pl010_modem_status(struct uart_amba_port *uap)
diff --git a/drivers/tty/serial/apbuart.c b/drivers/tty/serial/apbuart.c
index 450f4edfda0f..915ee4b0d594 100644
--- a/drivers/tty/serial/apbuart.c
+++ b/drivers/tty/serial/apbuart.c
@@ -122,36 +122,12 @@ static void apbuart_rx_chars(struct uart_port *port)
 
 static void apbuart_tx_chars(struct uart_port *port)
 {
-	struct circ_buf *xmit = &port->state->xmit;
-	int count;
-
-	if (port->x_char) {
-		UART_PUT_CHAR(port, port->x_char);
-		port->icount.tx++;
-		port->x_char = 0;
-		return;
-	}
-
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
-		apbuart_stop_tx(port);
-		return;
-	}
-
-	/* amba: fill FIFO */
-	count = port->fifosize >> 1;
-	do {
-		UART_PUT_CHAR(port, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	} while (--count > 0);
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
+	u8 ch;
 
-	if (uart_circ_empty(xmit))
-		apbuart_stop_tx(port);
+	uart_port_tx_limited(port, ch, port->fifosize >> 1,
+		true,
+		UART_PUT_CHAR(port, ch),
+		({}));
 }
 
 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 5d9737c2d1f2..62bc7244dc67 100644
--- a/drivers/tty/serial/bcm63xx_uart.c
+++ b/drivers/tty/serial/bcm63xx_uart.c
@@ -303,53 +303,24 @@ static void bcm_uart_do_rx(struct uart_port *port)
  */
 static void bcm_uart_do_tx(struct uart_port *port)
 {
-	struct circ_buf *xmit;
-	unsigned int val, max_count;
-
-	if (port->x_char) {
-		bcm_uart_writel(port, port->x_char, UART_FIFO_REG);
-		port->icount.tx++;
-		port->x_char = 0;
-		return;
-	}
-
-	if (uart_tx_stopped(port)) {
-		bcm_uart_stop_tx(port);
-		return;
-	}
-
-	xmit = &port->state->xmit;
-	if (uart_circ_empty(xmit))
-		goto txq_empty;
+	unsigned int val;
+	bool pending;
+	u8 ch;
 
 	val = bcm_uart_readl(port, UART_MCTL_REG);
 	val = (val & UART_MCTL_TXFIFOFILL_MASK) >> UART_MCTL_TXFIFOFILL_SHIFT;
-	max_count = port->fifosize - val;
-
-	while (max_count--) {
-		unsigned int c;
 
-		c = xmit->buf[xmit->tail];
-		bcm_uart_writel(port, c, UART_FIFO_REG);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
-
-	if (uart_circ_empty(xmit))
-		goto txq_empty;
-	return;
+	pending = uart_port_tx_limited(port, ch, port->fifosize - val,
+		true,
+		bcm_uart_writel(port, ch, UART_FIFO_REG),
+		({}));
+	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 ed0e763f622a..85ce1e9af44a 100644
--- a/drivers/tty/serial/mux.c
+++ b/drivers/tty/serial/mux.c
@@ -171,6 +171,13 @@ 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);
+}
+
 /**
  * mux_write - Write chars to the mux fifo.
  * @port: Ptr to the uart_port.
@@ -180,39 +187,13 @@ 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);
+	u8 ch;
 
-	if (uart_circ_empty(xmit))
-		mux_stop_tx(port);
+	uart_port_tx_limited(port, ch,
+		port->fifosize - UART_GET_FIFO_CNT(port),
+		true,
+		UART_PUT_CHAR(port, ch),
+		mux_tx_done(port));
 }
 
 /**
diff --git a/drivers/tty/serial/mvebu-uart.c b/drivers/tty/serial/mvebu-uart.c
index ba16e1da6bd3..7b566404cb33 100644
--- a/drivers/tty/serial/mvebu-uart.c
+++ b/drivers/tty/serial/mvebu-uart.c
@@ -335,40 +335,12 @@ static void mvebu_uart_rx_chars(struct uart_port *port, unsigned int status)
 
 static void mvebu_uart_tx_chars(struct uart_port *port, unsigned int status)
 {
-	struct circ_buf *xmit = &port->state->xmit;
-	unsigned int count;
-	unsigned int st;
-
-	if (port->x_char) {
-		writel(port->x_char, port->membase + UART_TSH(port));
-		port->icount.tx++;
-		port->x_char = 0;
-		return;
-	}
-
-	if (uart_circ_empty(xmit) || uart_tx_stopped(port)) {
-		mvebu_uart_stop_tx(port);
-		return;
-	}
-
-	for (count = 0; count < port->fifosize; count++) {
-		writel(xmit->buf[xmit->tail], port->membase + UART_TSH(port));
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		port->icount.tx++;
-
-		if (uart_circ_empty(xmit))
-			break;
-
-		st = readl(port->membase + UART_STAT);
-		if (st & STAT_TX_FIFO_FUL)
-			break;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(port);
+	u8 ch;
 
-	if (uart_circ_empty(xmit))
-		mvebu_uart_stop_tx(port);
+	uart_port_tx_limited(port, ch, port->fifosize,
+		!(readl(port->membase + UART_STAT) & STAT_TX_FIFO_FUL),
+		writel(ch, port->membase + UART_TSH(port)),
+		({}));
 }
 
 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 7d0d2718ef59..82d35dbbfa6c 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -347,34 +347,12 @@ static void serial_omap_put_char(struct uart_omap_port *up, unsigned char ch)
 
 static void transmit_chars(struct uart_omap_port *up, unsigned int lsr)
 {
-	struct circ_buf *xmit = &up->port.state->xmit;
-	int count;
+	u8 ch;
 
-	if (up->port.x_char) {
-		serial_omap_put_char(up, 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_omap_stop_tx(&up->port);
-		return;
-	}
-	count = up->port.fifosize / 4;
-	do {
-		serial_omap_put_char(up, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		up->port.icount.tx++;
-
-		if (uart_circ_empty(xmit))
-			break;
-	} while (--count > 0);
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(&up->port);
-
-	if (uart_circ_empty(xmit))
-		serial_omap_stop_tx(&up->port);
+	uart_port_tx_limited(&up->port, ch, up->port.fifosize / 4,
+		true,
+		serial_omap_put_char(up, ch),
+		({}));
 }
 
 static inline void serial_omap_enable_ier_thri(struct uart_omap_port *up)
diff --git a/drivers/tty/serial/pxa.c b/drivers/tty/serial/pxa.c
index 2d25231fad84..444fa4b654ac 100644
--- a/drivers/tty/serial/pxa.c
+++ b/drivers/tty/serial/pxa.c
@@ -174,35 +174,12 @@ static inline void receive_chars(struct uart_pxa_port *up, int *status)
 
 static void transmit_chars(struct uart_pxa_port *up)
 {
-	struct circ_buf *xmit = &up->port.state->xmit;
-	int count;
+	u8 ch;
 
-	if (up->port.x_char) {
-		serial_out(up, UART_TX, up->port.x_char);
-		up->port.icount.tx++;
-		up->port.x_char = 0;
-		return;
-	}
-	if (uart_circ_empty(xmit) || uart_tx_stopped(&up->port)) {
-		serial_pxa_stop_tx(&up->port);
-		return;
-	}
-
-	count = up->port.fifosize / 2;
-	do {
-		serial_out(up, UART_TX, xmit->buf[xmit->tail]);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		up->port.icount.tx++;
-		if (uart_circ_empty(xmit))
-			break;
-	} while (--count > 0);
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(&up->port);
-
-
-	if (uart_circ_empty(xmit))
-		serial_pxa_stop_tx(&up->port);
+	uart_port_tx_limited(&up->port, ch, up->port.fifosize / 2,
+		true,
+		serial_out(up, UART_TX, ch),
+		({}));
 }
 
 static void serial_pxa_start_tx(struct uart_port *port)
diff --git a/drivers/tty/serial/rp2.c b/drivers/tty/serial/rp2.c
index b81afb06f1f4..749b873a5d99 100644
--- a/drivers/tty/serial/rp2.c
+++ b/drivers/tty/serial/rp2.c
@@ -427,32 +427,13 @@ static void rp2_rx_chars(struct rp2_uart_port *up)
 
 static void rp2_tx_chars(struct rp2_uart_port *up)
 {
-	u16 max_tx = FIFO_SIZE - readw(up->base + RP2_TX_FIFO_COUNT);
-	struct circ_buf *xmit = &up->port.state->xmit;
+	u8 ch;
 
-	if (uart_tx_stopped(&up->port)) {
-		rp2_uart_stop_tx(&up->port);
-		return;
-	}
-
-	for (; max_tx != 0; max_tx--) {
-		if (up->port.x_char) {
-			writeb(up->port.x_char, up->base + RP2_DATA_BYTE);
-			up->port.x_char = 0;
-			up->port.icount.tx++;
-			continue;
-		}
-		if (uart_circ_empty(xmit)) {
-			rp2_uart_stop_tx(&up->port);
-			break;
-		}
-		writeb(xmit->buf[xmit->tail], up->base + RP2_DATA_BYTE);
-		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
-		up->port.icount.tx++;
-	}
-
-	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
-		uart_write_wakeup(&up->port);
+	uart_port_tx_limited(&up->port, ch,
+		FIFO_SIZE - readw(up->base + RP2_TX_FIFO_COUNT),
+		true,
+		writeb(ch, up->base + RP2_DATA_BYTE),
+		({}));
 }
 
 static void rp2_ch_interrupt(struct rp2_uart_port *up)
diff --git a/drivers/tty/serial/serial_txx9.c b/drivers/tty/serial/serial_txx9.c
index e12f1dc18c38..eab387b01e36 100644
--- a/drivers/tty/serial/serial_txx9.c
+++ b/drivers/tty/serial/serial_txx9.c
@@ -321,34 +321,12 @@ receive_chars(struct uart_port *up, unsigned int *status)
 
 static inline void transmit_chars(struct uart_port *up)
 {
-	struct circ_buf *xmit = &up->state->xmit;
-	int count;
+	u8 ch;
 
-	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);
+	uart_port_tx_limited(up, ch, TXX9_SIO_TX_FIFO,
+		true,
+		sio_out(up, TXX9_SITFIFO, ch),
+		({}));
 }
 
 static irqreturn_t serial_txx9_interrupt(int irq, void *dev_id)
diff --git a/drivers/tty/serial/sifive.c b/drivers/tty/serial/sifive.c
index 7fb6760b5c37..1f565a216e74 100644
--- a/drivers/tty/serial/sifive.c
+++ b/drivers/tty/serial/sifive.c
@@ -288,33 +288,12 @@ static void __ssp_transmit_char(struct sifive_serial_port *ssp, int ch)
  */
 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);
+	u8 ch;
 
-	if (uart_circ_empty(xmit))
-		sifive_serial_stop_tx(&ssp->port);
+	uart_port_tx_limited(&ssp->port, ch, SIFIVE_TX_FIFO_DEPTH,
+		true,
+		__ssp_transmit_char(ssp, ch),
+		({}));
 }
 
 /**
diff --git a/drivers/tty/serial/sprd_serial.c b/drivers/tty/serial/sprd_serial.c
index 342a87967631..3f34f7bb7700 100644
--- a/drivers/tty/serial/sprd_serial.c
+++ b/drivers/tty/serial/sprd_serial.c
@@ -626,35 +626,12 @@ static inline void sprd_rx(struct uart_port *port)
 
 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);
+	u8 ch;
 
-	if (uart_circ_empty(xmit))
-		sprd_stop_tx(port);
+	uart_port_tx_limited(port, ch, THLD_TX_EMPTY,
+		true,
+		serial_out(port, SPRD_TXD, ch),
+		({}));
 }
 
 /* this handles the interrupt from one port */
diff --git a/drivers/tty/serial/st-asc.c b/drivers/tty/serial/st-asc.c
index fcecea689a0d..5215e6910f68 100644
--- a/drivers/tty/serial/st-asc.c
+++ b/drivers/tty/serial/st-asc.c
@@ -237,50 +237,12 @@ 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);
+	u8 ch;
 
-	if (uart_circ_empty(xmit))
-		asc_disable_tx_interrupts(port);
+	uart_port_tx_limited(port, ch, asc_hw_txroom(port),
+		true,
+		asc_out(port, ASC_TXBUF, ch),
+		({}));
 }
 
 static void asc_receive_chars(struct uart_port *port)
-- 
2.37.3

Re: [PATCH v5 3/3] tty: serial: use uart_port_tx_limited()

[Ilpo Järvinen]

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

On Tue, 4 Oct 2022, Jiri Slaby (SUSE) wrote:

> uart_port_tx_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.
> 
> 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
> Signed-off-by: Jiri Slaby (SUSE) <jirislaby@kernel.org>

Perhaps you missed this one in the earlier mail. ...Anyway, here it's 
again:

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

-- 
 i.

Re: [PATCH v5 2/3] tty: serial: use uart_port_tx() helper

[Michael Walle]

From: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>

Hi Jiri,

> diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
> index bd07f79a2df9..a6b4d30c5888 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -824,30 +824,14 @@ static void atmel_rx_chars(struct uart_port *port)
>   */
>  static void atmel_tx_chars(struct uart_port *port)
>  {
> -	struct circ_buf *xmit = &port->state->xmit;
>  	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> +	bool pending;
> +	u8 ch;
>  
> -	if (port->x_char &&
> -	    (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY)) {
> -		atmel_uart_write_char(port, port->x_char);
> -		port->icount.tx++;
> -		port->x_char = 0;
> -	}
> -	if (uart_circ_empty(xmit) || uart_tx_stopped(port))
> -		return;
> -
> -	while (atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY) {
> -		atmel_uart_write_char(port, xmit->buf[xmit->tail]);
> -		xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
> -		port->icount.tx++;
> -		if (uart_circ_empty(xmit))
> -			break;
> -	}
> -
> -	if (uart_circ_chars_pending(xmit) < WAKEUP_CHARS)
> -		uart_write_wakeup(port);
> -
> -	if (!uart_circ_empty(xmit)) {
> +	pending = uart_port_tx(port, ch,
> +		atmel_uart_readl(port, ATMEL_US_CSR) & ATMEL_US_TXRDY,
> +		atmel_uart_write_char(port, ch));
> +	if (pending) {
>  		/* we still have characters to transmit, so we should continue
>  		 * transmitting them when TX is ready, regardless of
>  		 * mode or duplexity

This will break serial output for the userspace on my board
(arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt*dts). The uart_port_tx()
helper will call __port->ops->stop_tx(__port) if uart_circ_chars_pending()
returns 0. But the code above, doesn't do that. In fact, removing the
stop_tx() call in the helper macro, will fix the console output.

Any ideas how to fix that?

-michael

Re: [PATCH v5 2/3] tty: serial: use uart_port_tx() helper

[Jiri Slaby]

Hi,

On 21. 11. 22, 21:27, Michael Walle wrote:
> This will break serial output for the userspace on my board
> (arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt*dts). The uart_port_tx()
> helper will call __port->ops->stop_tx(__port) if uart_circ_chars_pending()
> returns 0. But the code above, doesn't do that. In fact, removing the
> stop_tx() call in the helper macro, will fix the console output.
> 
> Any ideas how to fix that?

Hm, so ATMEL_US_TXRDY is removed from tx_done_mask in stop_tx, but not 
added back in start_tx. So the tx interrupt is never handled (the tx 
tasklet is not scheduled to send the queue chars) in 
atmel_handle_transmit().

Any chance, the below fixes it?

diff --git a/drivers/tty/serial/atmel_serial.c 
b/drivers/tty/serial/atmel_serial.c
index 11bf2466390e..395370e0c77b 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -596,6 +596,8 @@ static void atmel_start_tx(struct uart_port *port)
                 /* re-enable PDC transmit */
                 atmel_uart_writel(port, ATMEL_PDC_PTCR, ATMEL_PDC_TXTEN);

+       atmel_port->tx_done_mask |= ATMEL_US_TXRDY;
+
         /* Enable interrupts */
         atmel_uart_writel(port, ATMEL_US_IER, atmel_port->tx_done_mask);


thanks,
-- 
js

Re: [PATCH v5 2/3] tty: serial: use uart_port_tx() helper

[Michael Walle]

Hi,

Am 2022-11-22 08:02, schrieb Jiri Slaby:
> On 21. 11. 22, 21:27, Michael Walle wrote:
>> This will break serial output for the userspace on my board
>> (arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt*dts). The 
>> uart_port_tx()
>> helper will call __port->ops->stop_tx(__port) if 
>> uart_circ_chars_pending()
>> returns 0. But the code above, doesn't do that. In fact, removing the
>> stop_tx() call in the helper macro, will fix the console output.
>> 
>> Any ideas how to fix that?
> 
> Hm, so ATMEL_US_TXRDY is removed from tx_done_mask in stop_tx, but not
> added back in start_tx. So the tx interrupt is never handled (the tx
> tasklet is not scheduled to send the queue chars) in
> atmel_handle_transmit().
> 
> Any chance, the below fixes it?
> 
> diff --git a/drivers/tty/serial/atmel_serial.c
> b/drivers/tty/serial/atmel_serial.c
> index 11bf2466390e..395370e0c77b 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -596,6 +596,8 @@ static void atmel_start_tx(struct uart_port *port)
>                 /* re-enable PDC transmit */
>                 atmel_uart_writel(port, ATMEL_PDC_PTCR, 
> ATMEL_PDC_TXTEN);
> 
> +       atmel_port->tx_done_mask |= ATMEL_US_TXRDY;
> +
>         /* Enable interrupts */
>         atmel_uart_writel(port, ATMEL_US_IER, 
> atmel_port->tx_done_mask);
> 
> 
> thanks,

Unfortunately, that doesn't help. Btw, some characters are transmitted:


[    6.219356] Key type dns_resolver registered
[    6.223679] Registering SWP/SWPB emulation handler
[    6.247530] Loading compiled-in X.509 certificates
[    6.288467] Freeing unused kernel image (initmem) memory: 1024K
[    6.297789] Run /init as init process
WbSOROSOSOSOSOStarting linuxptp system clock synchronization: O

-michael

Re: [PATCH v5 2/3] tty: serial: use uart_port_tx() helper

[Michael Walle]

Am 2022-11-22 09:09, schrieb Michael Walle:
> Hi,
> 
> Am 2022-11-22 08:02, schrieb Jiri Slaby:
>> On 21. 11. 22, 21:27, Michael Walle wrote:
>>> This will break serial output for the userspace on my board
>>> (arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt*dts). The 
>>> uart_port_tx()
>>> helper will call __port->ops->stop_tx(__port) if 
>>> uart_circ_chars_pending()
>>> returns 0. But the code above, doesn't do that. In fact, removing the
>>> stop_tx() call in the helper macro, will fix the console output.
>>> 
>>> Any ideas how to fix that?
>> 
>> Hm, so ATMEL_US_TXRDY is removed from tx_done_mask in stop_tx, but not
>> added back in start_tx. So the tx interrupt is never handled (the tx
>> tasklet is not scheduled to send the queue chars) in
>> atmel_handle_transmit().
>> 
>> Any chance, the below fixes it?
>> 
>> diff --git a/drivers/tty/serial/atmel_serial.c
>> b/drivers/tty/serial/atmel_serial.c
>> index 11bf2466390e..395370e0c77b 100644
>> --- a/drivers/tty/serial/atmel_serial.c
>> +++ b/drivers/tty/serial/atmel_serial.c
>> @@ -596,6 +596,8 @@ static void atmel_start_tx(struct uart_port *port)
>>                 /* re-enable PDC transmit */
>>                 atmel_uart_writel(port, ATMEL_PDC_PTCR, 
>> ATMEL_PDC_TXTEN);
>> 
>> +       atmel_port->tx_done_mask |= ATMEL_US_TXRDY;
>> +
>>         /* Enable interrupts */
>>         atmel_uart_writel(port, ATMEL_US_IER, 
>> atmel_port->tx_done_mask);
>> 
>> 
>> thanks,
> 
> Unfortunately, that doesn't help. Btw, some characters are transmitted:
> 
> 
> [    6.219356] Key type dns_resolver registered
> [    6.223679] Registering SWP/SWPB emulation handler
> [    6.247530] Loading compiled-in X.509 certificates
> [    6.288467] Freeing unused kernel image (initmem) memory: 1024K
> [    6.297789] Run /init as init process
> WbSOROSOSOSOSOStarting linuxptp system clock synchronization: O
> 
> -michael

But you made me look at atmel_stop_tx() and there is this:

        /*
	* Disable the transmitter.
	* This is mandatory when DMA is used, otherwise the DMA buffer
	* is fully transmitted.
	*/
	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS);

Removing that write, will also fix the problem. Could it be, that
the transmit is still active (via DMA) but the driver will call
tx_stop() and then stop the transmission in the background?

-michael

Re: [PATCH v5 2/3] tty: serial: use uart_port_tx() helper

[Jiri Slaby]

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

On 22. 11. 22, 9:18, Michael Walle wrote:
> Am 2022-11-22 09:09, schrieb Michael Walle:
>> Hi,
>>
>> Am 2022-11-22 08:02, schrieb Jiri Slaby:
>>> On 21. 11. 22, 21:27, Michael Walle wrote:
>>>> This will break serial output for the userspace on my board
>>>> (arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt*dts). The 
>>>> uart_port_tx()
>>>> helper will call __port->ops->stop_tx(__port) if 
>>>> uart_circ_chars_pending()
>>>> returns 0. But the code above, doesn't do that. In fact, removing the
>>>> stop_tx() call in the helper macro, will fix the console output.
>>>>
>>>> Any ideas how to fix that?
>>>
>>> Hm, so ATMEL_US_TXRDY is removed from tx_done_mask in stop_tx, but not
>>> added back in start_tx. So the tx interrupt is never handled (the tx
>>> tasklet is not scheduled to send the queue chars) in
>>> atmel_handle_transmit().
>>>
>>> Any chance, the below fixes it?
>>>
>>> diff --git a/drivers/tty/serial/atmel_serial.c
>>> b/drivers/tty/serial/atmel_serial.c
>>> index 11bf2466390e..395370e0c77b 100644
>>> --- a/drivers/tty/serial/atmel_serial.c
>>> +++ b/drivers/tty/serial/atmel_serial.c
>>> @@ -596,6 +596,8 @@ static void atmel_start_tx(struct uart_port *port)
>>>                 /* re-enable PDC transmit */
>>>                 atmel_uart_writel(port, ATMEL_PDC_PTCR, 
>>> ATMEL_PDC_TXTEN);
>>>
>>> +       atmel_port->tx_done_mask |= ATMEL_US_TXRDY;
>>> +
>>>         /* Enable interrupts */
>>>         atmel_uart_writel(port, ATMEL_US_IER, atmel_port->tx_done_mask);
>>>
>>>
>>> thanks,
>>
>> Unfortunately, that doesn't help. Btw, some characters are transmitted:
>>
>>
>> [    6.219356] Key type dns_resolver registered
>> [    6.223679] Registering SWP/SWPB emulation handler
>> [    6.247530] Loading compiled-in X.509 certificates
>> [    6.288467] Freeing unused kernel image (initmem) memory: 1024K
>> [    6.297789] Run /init as init process
>> WbSOROSOSOSOSOStarting linuxptp system clock synchronization: O
>>
>> -michael
> 
> But you made me look at atmel_stop_tx() and there is this:
> 
>         /*
>      * Disable the transmitter.
>      * This is mandatory when DMA is used, otherwise the DMA buffer
>      * is fully transmitted.
>      */
>      atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS);
> 
> Removing that write, will also fix the problem. Could it be, that
> the transmit is still active (via DMA) but the driver will call
> tx_stop() and then stop the transmission in the background?

Yes, that was exactly the next step to try. The datasheet doesn't tell 
much what happens when TXDIS is written while the characters are 
transmitted. So can you give the attached patch a try?

thanks,
-- 
js

[-- Attachment #2: 0001-atmel-fix.patch --]
[-- Type: text/x-patch, Size: 2779 bytes --]

From 2dcc79384b97a2176bbf84ea634aae4a26efd0b4 Mon Sep 17 00:00:00 2001
From: "Jiri Slaby (SUSE)" <jirislaby@kernel.org>
Date: Tue, 22 Nov 2022 09:05:51 +0100
Subject: [PATCH] atmel: fix

---
 drivers/tty/serial/atmel_serial.c | 40 ++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 16 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 11bf2466390e..bb44f297a4fc 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -552,19 +552,23 @@ static u_int atmel_get_mctrl(struct uart_port *port)
 static void atmel_stop_tx(struct uart_port *port)
 {
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+	bool is_pdc = atmel_use_pdc_tx(port);
+	bool is_dma = is_pdc || atmel_use_dma_tx(port);
 
-	if (atmel_use_pdc_tx(port)) {
+	if (is_pdc) {
 		/* disable PDC transmit */
 		atmel_uart_writel(port, ATMEL_PDC_PTCR, ATMEL_PDC_TXTDIS);
 	}
 
-	/*
-	 * Disable the transmitter.
-	 * This is mandatory when DMA is used, otherwise the DMA buffer
-	 * is fully transmitted.
-	 */
-	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS);
-	atmel_port->tx_stopped = true;
+	if (is_dma) {
+		/*
+		 * Disable the transmitter.
+		 * This is mandatory when DMA is used, otherwise the DMA buffer
+		 * is fully transmitted.
+		 */
+		atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS);
+		atmel_port->tx_stopped = true;
+	}
 
 	/* Disable interrupts */
 	atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
@@ -581,27 +585,31 @@ static void atmel_stop_tx(struct uart_port *port)
 static void atmel_start_tx(struct uart_port *port)
 {
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+	bool is_pdc = atmel_use_pdc_tx(port);
+	bool is_dma = is_pdc || atmel_use_dma_tx(port);
 
-	if (atmel_use_pdc_tx(port) && (atmel_uart_readl(port, ATMEL_PDC_PTSR)
+	if (is_pdc && (atmel_uart_readl(port, ATMEL_PDC_PTSR)
 				       & ATMEL_PDC_TXTEN))
 		/* The transmitter is already running.  Yes, we
 		   really need this.*/
 		return;
 
-	if (atmel_use_pdc_tx(port) || atmel_use_dma_tx(port))
-		if (atmel_uart_is_half_duplex(port))
-			atmel_stop_rx(port);
+	if (id_dma && atmel_uart_is_half_duplex(port))
+		atmel_stop_rx(port);
 
-	if (atmel_use_pdc_tx(port))
+	if (is_pdc) {
 		/* re-enable PDC transmit */
 		atmel_uart_writel(port, ATMEL_PDC_PTCR, ATMEL_PDC_TXTEN);
+	}
 
 	/* Enable interrupts */
 	atmel_uart_writel(port, ATMEL_US_IER, atmel_port->tx_done_mask);
 
-	/* re-enable the transmitter */
-	atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
-	atmel_port->tx_stopped = false;
+	if (is_dma) {
+		/* re-enable the transmitter */
+		atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXEN);
+		atmel_port->tx_stopped = false;
+	}
 }
 
 /*
-- 
2.38.1

Re: [PATCH v5 2/3] tty: serial: use uart_port_tx() helper

[Jiri Slaby]

On 22. 11. 22, 9:23, Jiri Slaby wrote:
> On 22. 11. 22, 9:18, Michael Walle wrote:
>> Am 2022-11-22 09:09, schrieb Michael Walle:
>>> Hi,
>>>
>>> Am 2022-11-22 08:02, schrieb Jiri Slaby:
>>>> On 21. 11. 22, 21:27, Michael Walle wrote:
>>>>> This will break serial output for the userspace on my board
>>>>> (arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt*dts). The 
>>>>> uart_port_tx()
>>>>> helper will call __port->ops->stop_tx(__port) if 
>>>>> uart_circ_chars_pending()
>>>>> returns 0. But the code above, doesn't do that. In fact, removing the
>>>>> stop_tx() call in the helper macro, will fix the console output.
>>>>>
>>>>> Any ideas how to fix that?
>>>>
>>>> Hm, so ATMEL_US_TXRDY is removed from tx_done_mask in stop_tx, but not
>>>> added back in start_tx. So the tx interrupt is never handled (the tx
>>>> tasklet is not scheduled to send the queue chars) in
>>>> atmel_handle_transmit().
>>>>
>>>> Any chance, the below fixes it?
>>>>
>>>> diff --git a/drivers/tty/serial/atmel_serial.c
>>>> b/drivers/tty/serial/atmel_serial.c
>>>> index 11bf2466390e..395370e0c77b 100644
>>>> --- a/drivers/tty/serial/atmel_serial.c
>>>> +++ b/drivers/tty/serial/atmel_serial.c
>>>> @@ -596,6 +596,8 @@ static void atmel_start_tx(struct uart_port *port)
>>>>                 /* re-enable PDC transmit */
>>>>                 atmel_uart_writel(port, ATMEL_PDC_PTCR, 
>>>> ATMEL_PDC_TXTEN);
>>>>
>>>> +       atmel_port->tx_done_mask |= ATMEL_US_TXRDY;
>>>> +
>>>>         /* Enable interrupts */
>>>>         atmel_uart_writel(port, ATMEL_US_IER, 
>>>> atmel_port->tx_done_mask);
>>>>
>>>>
>>>> thanks,
>>>
>>> Unfortunately, that doesn't help. Btw, some characters are transmitted:
>>>
>>>
>>> [    6.219356] Key type dns_resolver registered
>>> [    6.223679] Registering SWP/SWPB emulation handler
>>> [    6.247530] Loading compiled-in X.509 certificates
>>> [    6.288467] Freeing unused kernel image (initmem) memory: 1024K
>>> [    6.297789] Run /init as init process
>>> WbSOROSOSOSOSOStarting linuxptp system clock synchronization: O
>>>
>>> -michael
>>
>> But you made me look at atmel_stop_tx() and there is this:
>>
>>         /*
>>      * Disable the transmitter.
>>      * This is mandatory when DMA is used, otherwise the DMA buffer
>>      * is fully transmitted.
>>      */
>>      atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS);
>>
>> Removing that write, will also fix the problem. Could it be, that
>> the transmit is still active (via DMA) but the driver will call
>> tx_stop() and then stop the transmission in the background?
> 
> Yes, that was exactly the next step to try. The datasheet doesn't tell 
> much what happens when TXDIS is written while the characters are 
> transmitted.

Side note: your usart doesn't use dma. It's PIO (hence all that 
uart_tx_helper()). And the attached patch doesn't touch TXDIS for 
non-DMA case. I.e. it should transmit the final character (and nothing 
more).

-- 
js

Re: [PATCH v5 2/3] tty: serial: use uart_port_tx() helper

[Michael Walle]

Am 2022-11-22 09:25, schrieb Jiri Slaby:
> On 22. 11. 22, 9:23, Jiri Slaby wrote:
>> On 22. 11. 22, 9:18, Michael Walle wrote:
>>> Am 2022-11-22 09:09, schrieb Michael Walle:
>>>> Hi,
>>>> 
>>>> Am 2022-11-22 08:02, schrieb Jiri Slaby:
>>>>> On 21. 11. 22, 21:27, Michael Walle wrote:
>>>>>> This will break serial output for the userspace on my board
>>>>>> (arch/arm/boot/dts/lan966x-kontron-kswitch-d10-mmt*dts). The 
>>>>>> uart_port_tx()
>>>>>> helper will call __port->ops->stop_tx(__port) if 
>>>>>> uart_circ_chars_pending()
>>>>>> returns 0. But the code above, doesn't do that. In fact, removing 
>>>>>> the
>>>>>> stop_tx() call in the helper macro, will fix the console output.
>>>>>> 
>>>>>> Any ideas how to fix that?
>>>>> 
>>>>> Hm, so ATMEL_US_TXRDY is removed from tx_done_mask in stop_tx, but 
>>>>> not
>>>>> added back in start_tx. So the tx interrupt is never handled (the 
>>>>> tx
>>>>> tasklet is not scheduled to send the queue chars) in
>>>>> atmel_handle_transmit().
>>>>> 
>>>>> Any chance, the below fixes it?
>>>>> 
>>>>> diff --git a/drivers/tty/serial/atmel_serial.c
>>>>> b/drivers/tty/serial/atmel_serial.c
>>>>> index 11bf2466390e..395370e0c77b 100644
>>>>> --- a/drivers/tty/serial/atmel_serial.c
>>>>> +++ b/drivers/tty/serial/atmel_serial.c
>>>>> @@ -596,6 +596,8 @@ static void atmel_start_tx(struct uart_port 
>>>>> *port)
>>>>>                 /* re-enable PDC transmit */
>>>>>                 atmel_uart_writel(port, ATMEL_PDC_PTCR, 
>>>>> ATMEL_PDC_TXTEN);
>>>>> 
>>>>> +       atmel_port->tx_done_mask |= ATMEL_US_TXRDY;
>>>>> +
>>>>>         /* Enable interrupts */
>>>>>         atmel_uart_writel(port, ATMEL_US_IER, 
>>>>> atmel_port->tx_done_mask);
>>>>> 
>>>>> 
>>>>> thanks,
>>>> 
>>>> Unfortunately, that doesn't help. Btw, some characters are 
>>>> transmitted:
>>>> 
>>>> 
>>>> [    6.219356] Key type dns_resolver registered
>>>> [    6.223679] Registering SWP/SWPB emulation handler
>>>> [    6.247530] Loading compiled-in X.509 certificates
>>>> [    6.288467] Freeing unused kernel image (initmem) memory: 1024K
>>>> [    6.297789] Run /init as init process
>>>> WbSOROSOSOSOSOStarting linuxptp system clock synchronization: O
>>>> 
>>>> -michael
>>> 
>>> But you made me look at atmel_stop_tx() and there is this:
>>> 
>>>         /*
>>>      * Disable the transmitter.
>>>      * This is mandatory when DMA is used, otherwise the DMA buffer
>>>      * is fully transmitted.
>>>      */
>>>      atmel_uart_writel(port, ATMEL_US_CR, ATMEL_US_TXDIS);
>>> 
>>> Removing that write, will also fix the problem. Could it be, that
>>> the transmit is still active (via DMA) but the driver will call
>>> tx_stop() and then stop the transmission in the background?
>> 
>> Yes, that was exactly the next step to try. The datasheet doesn't tell 
>> much what happens when TXDIS is written while the characters are 
>> transmitted.
> 
> Side note: your usart doesn't use dma. It's PIO (hence all that
> uart_tx_helper()). And the attached patch doesn't touch TXDIS for
> non-DMA case. I.e. it should transmit the final character (and nothing
> more).

ok ;)

expect from "s/id_dma/is_dma/", this patch works. thanks!

-michael