2022-09-20 05:52:29

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH v4 00/10] tty: TX helpers

This series introduces uart_port_tx() + uart_port_tx_limited() TX
helpers. See PATCH 8/10 for the details. Comments welcome.

First the series performs simple cleanups, so that the later patches are
easier to follow.

Then it switches drivers to use them. First, to uart_port_tx() in 9/10
and then uart_port_tx_limited() in 10/10.

The diffstat of patches 9+10 is as follows:
26 files changed, 145 insertions(+), 740 deletions(-)
which appears to be nice.

Cc: Arnd Bergmann <[email protected]>
Cc: Johan Hovold <[email protected]>
Cc: "Russell King (Oracle)" <[email protected]>
Cc: Ilpo Järvinen <[email protected]>

Jiri Slaby (10):
tty: serial: move and cleanup vt8500_tx_empty()
tty: serial: clean up stop-tx part in altera_uart_tx_chars()
tty: serial: altera_uart_{r,t}x_chars() need only uart_port
tty: serial: extract lqasc_tx_ready() from lqasc_tx_chars()
tty: serial: extract tx_ready() from __serial_lpc32xx_tx()
tty: serial: switch mpc52xx_uart_int_{r,t}x_chars() to bool
tty: serial: extract serial_omap_put_char() from transmit_chars()
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 | 40 +++--------
drivers/tty/serial/altera_uart.c | 41 +++--------
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 | 44 +++---------
drivers/tty/serial/lpc32xx_hs.c | 39 +++--------
drivers/tty/serial/mcf.c | 28 ++------
drivers/tty/serial/mpc52xx_uart.c | 49 +++----------
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 | 46 ++++---------
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 | 41 +++--------
include/linux/serial_core.h | 80 ++++++++++++++++++++++
28 files changed, 264 insertions(+), 765 deletions(-)

--
2.37.3


2022-09-20 05:52:54

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH v4 02/10] tty: serial: clean up stop-tx part in altera_uart_tx_chars()

The "stop TX" path in altera_uart_tx_chars() is open-coded, so:
* use uart_circ_empty() to check if the buffer is empty, and
* when true, call altera_uart_stop_tx().

Cc: Tobias Klauser <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
---

Notes:
[v4] this is new in v4 -- extracted as a separate change

drivers/tty/serial/altera_uart.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index a38db2cb8dc1..4170e66601ec 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -272,10 +272,8 @@ static void altera_uart_tx_chars(struct altera_uart *pp)
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);
- }
+ if (uart_circ_empty(xmit))
+ altera_uart_stop_tx(port);
}

static irqreturn_t altera_uart_interrupt(int irq, void *data)
--
2.37.3

2022-09-20 05:53:41

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH v4 04/10] tty: serial: extract lqasc_tx_ready() from lqasc_tx_chars()

The condition in lqasc_tx_chars()'s loop is barely readable. Extract it
to a separate function. This will make the cleanup in the next patches
easier too.

(Put it before lqasc_start_tx(), so that we can use it there later.)

Signed-off-by: Jiri Slaby <[email protected]>
---

Notes:
[v4] this is new in v4 -- extracted as a separate change

drivers/tty/serial/lantiq.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
index 6637b3caa6b7..6da1b7496c6c 100644
--- a/drivers/tty/serial/lantiq.c
+++ b/drivers/tty/serial/lantiq.c
@@ -139,6 +139,13 @@ 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 void
lqasc_start_tx(struct uart_port *port)
{
@@ -228,8 +235,7 @@ lqasc_tx_chars(struct uart_port *port)
return;
}

- while (((__raw_readl(port->membase + LTQ_ASC_FSTAT) &
- ASCFSTAT_TXFREEMASK) >> ASCFSTAT_TXFREEOFF) != 0) {
+ while (lqasc_tx_ready(port)) {
if (port->x_char) {
writeb(port->x_char, port->membase + LTQ_ASC_TBUF);
port->icount.tx++;
--
2.37.3

2022-09-20 06:02:06

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH v4 03/10] tty: serial: altera_uart_{r,t}x_chars() need only uart_port

Both altera_uart_{r,t}x_chars() need only uart_port, not altera_uart. So
pass the former from altera_uart_interrupt() directly.

Apart it maybe saves a dereference, this makes the transition of
altera_uart_tx_chars() easier to follow in the next patch.

Cc: Tobias Klauser <[email protected]>
Signed-off-by: Jiri Slaby <[email protected]>
---

Notes:
[v4] this is new in v4 -- extracted as a separate change

drivers/tty/serial/altera_uart.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/altera_uart.c b/drivers/tty/serial/altera_uart.c
index 4170e66601ec..82f2790de28d 100644
--- a/drivers/tty/serial/altera_uart.c
+++ b/drivers/tty/serial/altera_uart.c
@@ -199,9 +199,8 @@ static void altera_uart_set_termios(struct uart_port *port,
*/
}

-static void altera_uart_rx_chars(struct altera_uart *pp)
+static void altera_uart_rx_chars(struct uart_port *port)
{
- struct uart_port *port = &pp->port;
unsigned char ch, flag;
unsigned short status;

@@ -246,9 +245,8 @@ 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)
+static void altera_uart_tx_chars(struct uart_port *port)
{
- struct uart_port *port = &pp->port;
struct circ_buf *xmit = &port->state->xmit;

if (port->x_char) {
@@ -286,9 +284,9 @@ static irqreturn_t altera_uart_interrupt(int irq, void *data)

spin_lock(&port->lock);
if (isr & ALTERA_UART_STATUS_RRDY_MSK)
- altera_uart_rx_chars(pp);
+ altera_uart_rx_chars(port);
if (isr & ALTERA_UART_STATUS_TRDY_MSK)
- altera_uart_tx_chars(pp);
+ altera_uart_tx_chars(port);
spin_unlock(&port->lock);

return IRQ_RETVAL(isr);
--
2.37.3

2022-09-20 08:13:35

by Jiri Slaby

[permalink] [raw]
Subject: [PATCH v4 10/10] tty: serial: use uart_port_tx_limited()

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.

Signed-off-by: Jiri Slaby <[email protected]>
Cc: Russell King <[email protected]>
Cc: Florian Fainelli <[email protected]>
Cc: [email protected]
Cc: "Pali Rohár" <[email protected]>
Cc: Kevin Cernekee <[email protected]>
Cc: Palmer Dabbelt <[email protected]>
Cc: Paul Walmsley <[email protected]>
Cc: Orson Zhai <[email protected]>
Cc: Baolin Wang <[email protected]>
Cc: Chunyan Zhang <[email protected]>
Cc: Patrice Chotard <[email protected]>
Cc: [email protected]
---

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 | 40 ++++++-----------------
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, 87 insertions(+), 421 deletions(-)

diff --git a/drivers/tty/serial/21285.c b/drivers/tty/serial/21285.c
index 2f17bf4b221e..84c1e3e365c8 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 23f339757894..f224f5141726 100644
--- a/drivers/tty/serial/altera_jtaguart.c
+++ b/drivers/tty/serial/altera_jtaguart.c
@@ -137,39 +137,17 @@ 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 space;
+ u8 ch;

- 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);
- }
- }
+ space = readl(port->membase + ALTERA_JTAGUART_CONTROL_REG);
+ space &= ALTERA_JTAGUART_CONTROL_WSPACE_MSK;
+ space >>= ALTERA_JTAGUART_CONTROL_WSPACE_OFF;

- if (pending == 0)
- altera_jtaguart_stop_tx(port);
+ uart_port_tx_limited(port, ch, space,
+ 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 b7b76e49115e..ccea746e1214 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -348,34 +348,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 4761f172103a..64cfd455f556 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

2022-09-20 08:15:31

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH v4 04/10] tty: serial: extract lqasc_tx_ready() from lqasc_tx_chars()

On Tue, 20 Sep 2022, Jiri Slaby wrote:

> The condition in lqasc_tx_chars()'s loop is barely readable. Extract it
> to a separate function. This will make the cleanup in the next patches
> easier too.
>
> (Put it before lqasc_start_tx(), so that we can use it there later.)
>
> Signed-off-by: Jiri Slaby <[email protected]>
> ---
>
> Notes:
> [v4] this is new in v4 -- extracted as a separate change
>
> drivers/tty/serial/lantiq.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/lantiq.c b/drivers/tty/serial/lantiq.c
> index 6637b3caa6b7..6da1b7496c6c 100644
> --- a/drivers/tty/serial/lantiq.c
> +++ b/drivers/tty/serial/lantiq.c
> @@ -139,6 +139,13 @@ 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;

FIELD_GET(ASCFSTAT_TXFREEMASK, fstat)

> +}
> +
> static void
> lqasc_start_tx(struct uart_port *port)
> {
> @@ -228,8 +235,7 @@ lqasc_tx_chars(struct uart_port *port)
> return;
> }
>
> - while (((__raw_readl(port->membase + LTQ_ASC_FSTAT) &
> - ASCFSTAT_TXFREEMASK) >> ASCFSTAT_TXFREEOFF) != 0) {
> + while (lqasc_tx_ready(port)) {
> if (port->x_char) {
> writeb(port->x_char, port->membase + LTQ_ASC_TBUF);
> port->icount.tx++;

There's similar construct in lqasc_console_putchar() that could take
advantage of the same helper.

With FIELD_GET + the other place using the new helper, ASCFSTAT_TXFREEOFF
can be dropped as well.


--
i.

2022-09-20 08:16:13

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH v4 03/10] tty: serial: altera_uart_{r,t}x_chars() need only uart_port

On Tue, 20 Sep 2022, Jiri Slaby wrote:

> Both altera_uart_{r,t}x_chars() need only uart_port, not altera_uart. So
> pass the former from altera_uart_interrupt() directly.
>
> Apart it maybe saves a dereference, this makes the transition of
> altera_uart_tx_chars() easier to follow in the next patch.
>
> Cc: Tobias Klauser <[email protected]>
> Signed-off-by: Jiri Slaby <[email protected]>

Reviewed-by: Ilpo J?rvinen <[email protected]>

--
i.

2022-09-20 08:30:28

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH v4 02/10] tty: serial: clean up stop-tx part in altera_uart_tx_chars()

On Tue, 20 Sep 2022, Jiri Slaby wrote:

> The "stop TX" path in altera_uart_tx_chars() is open-coded, so:
> * use uart_circ_empty() to check if the buffer is empty, and
> * when true, call altera_uart_stop_tx().
>
> Cc: Tobias Klauser <[email protected]>
> Signed-off-by: Jiri Slaby <[email protected]>

Reviewed-by: Ilpo J?rvinen <[email protected]>

--
i.

2022-09-20 09:34:05

by Ilpo Järvinen

[permalink] [raw]
Subject: Re: [PATCH v4 10/10] tty: serial: use uart_port_tx_limited()

On Tue, 20 Sep 2022, Jiri Slaby 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.
>
> Signed-off-by: Jiri Slaby <[email protected]>
> Cc: Russell King <[email protected]>
> Cc: Florian Fainelli <[email protected]>
> Cc: [email protected]
> Cc: "Pali Roh?r" <[email protected]>
> Cc: Kevin Cernekee <[email protected]>
> Cc: Palmer Dabbelt <[email protected]>
> Cc: Paul Walmsley <[email protected]>
> Cc: Orson Zhai <[email protected]>
> Cc: Baolin Wang <[email protected]>
> Cc: Chunyan Zhang <[email protected]>
> Cc: Patrice Chotard <[email protected]>
> Cc: [email protected]

Reviewed-by: Ilpo J?rvinen <[email protected]>

One improvement suggestion below.

> diff --git a/drivers/tty/serial/altera_jtaguart.c b/drivers/tty/serial/altera_jtaguart.c
> index 23f339757894..f224f5141726 100644
> --- a/drivers/tty/serial/altera_jtaguart.c
> +++ b/drivers/tty/serial/altera_jtaguart.c
> @@ -137,39 +137,17 @@ 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 space;
> + u8 ch;
>
> - 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);
> - }
> - }
> + space = readl(port->membase + ALTERA_JTAGUART_CONTROL_REG);
> + space &= ALTERA_JTAGUART_CONTROL_WSPACE_MSK;
> + space >>= ALTERA_JTAGUART_CONTROL_WSPACE_OFF;

This is FIELD_GET(ALTERA_JTAGUART_CONTROL_WSPACE_MSK, ...) & then allows
killing ALTERA_JTAGUART_CONTROL_WSPACE_OFF. I'd probably do it in a
separate patch though.


--
i.

2022-09-20 11:11:10

by Jiri Slaby

[permalink] [raw]
Subject: Re: [PATCH v4 10/10] tty: serial: use uart_port_tx_limited()

On 20. 09. 22, 11:19, Ilpo Järvinen wrote:
> One improvement suggestion below.
>
>> diff --git a/drivers/tty/serial/altera_jtaguart.c b/drivers/tty/serial/altera_jtaguart.c
>> index 23f339757894..f224f5141726 100644
>> --- a/drivers/tty/serial/altera_jtaguart.c
>> +++ b/drivers/tty/serial/altera_jtaguart.c
>> @@ -137,39 +137,17 @@ static void altera_jtaguart_rx_chars(struct altera_jtaguart *pp)
>> static void altera_jtaguart_tx_chars(struct altera_jtaguart *pp)
...
>> + space = readl(port->membase + ALTERA_JTAGUART_CONTROL_REG);
>> + space &= ALTERA_JTAGUART_CONTROL_WSPACE_MSK;
>> + space >>= ALTERA_JTAGUART_CONTROL_WSPACE_OFF;
>
> This is FIELD_GET(ALTERA_JTAGUART_CONTROL_WSPACE_MSK, ...) & then allows
> killing ALTERA_JTAGUART_CONTROL_WSPACE_OFF. I'd probably do it in a
> separate patch though.

Right. Prepared for v5.

thanks,
--
js
suse labs

2022-09-20 15:10:16

by Tobias Klauser

[permalink] [raw]
Subject: Re: [PATCH v4 02/10] tty: serial: clean up stop-tx part in altera_uart_tx_chars()

On 2022-09-20 at 07:20:42 +0200, Jiri Slaby <[email protected]> wrote:
> The "stop TX" path in altera_uart_tx_chars() is open-coded, so:
> * use uart_circ_empty() to check if the buffer is empty, and
> * when true, call altera_uart_stop_tx().
>
> Cc: Tobias Klauser <[email protected]>
> Signed-off-by: Jiri Slaby <[email protected]>

Acked-by: Tobias Klauser <[email protected]>

2022-09-20 15:40:38

by Tobias Klauser

[permalink] [raw]
Subject: Re: [PATCH v4 03/10] tty: serial: altera_uart_{r,t}x_chars() need only uart_port

On 2022-09-20 at 07:20:43 +0200, Jiri Slaby <[email protected]> wrote:
> Both altera_uart_{r,t}x_chars() need only uart_port, not altera_uart. So
> pass the former from altera_uart_interrupt() directly.
>
> Apart it maybe saves a dereference, this makes the transition of
> altera_uart_tx_chars() easier to follow in the next patch.
>
> Cc: Tobias Klauser <[email protected]>
> Signed-off-by: Jiri Slaby <[email protected]>

Acked-by: Tobias Klauser <[email protected]>

2022-09-22 15:03:16

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v4 00/10] tty: TX helpers

On Tue, Sep 20, 2022 at 07:20:40AM +0200, Jiri Slaby wrote:
> This series introduces uart_port_tx() + uart_port_tx_limited() TX
> helpers. See PATCH 8/10 for the details. Comments welcome.
>
> First the series performs simple cleanups, so that the later patches are
> easier to follow.
>
> Then it switches drivers to use them. First, to uart_port_tx() in 9/10
> and then uart_port_tx_limited() in 10/10.
>
> The diffstat of patches 9+10 is as follows:
> 26 files changed, 145 insertions(+), 740 deletions(-)
> which appears to be nice.

I've queued up the first 7 patches here, as they were the preparation /
cleanup patches and deserved to go in no matter what.

thanks,

greg k-h