When x_char is to be sent, the TX path overwrites whatever is in the
circular buffer at offset 0 with x_char and sends it using
pch_uart_hal_write(). I don't understand how this was supposed to work
if xmit->buf[0] already contained some character. It must have been
lost.
Remove this whole pop_tx_x() concept and do the work directly in the
callers. (Without printing anything using dev_dbg().)
Cc: <[email protected]>
Fixes: 3c6a483275f4 (Serial: EG20T: add PCH_UART driver)
Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/serial/pch_uart.c | 27 +++++++--------------------
1 file changed, 7 insertions(+), 20 deletions(-)
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index f872613a5e83..6cb631487383 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -624,22 +624,6 @@ static int push_rx(struct eg20t_port *priv, const unsigned char *buf,
return 0;
}
-static int pop_tx_x(struct eg20t_port *priv, unsigned char *buf)
-{
- int ret = 0;
- struct uart_port *port = &priv->port;
-
- if (port->x_char) {
- dev_dbg(priv->port.dev, "%s:X character send %02x (%lu)\n",
- __func__, port->x_char, jiffies);
- buf[0] = port->x_char;
- port->x_char = 0;
- ret = 1;
- }
-
- return ret;
-}
-
static int dma_push_rx(struct eg20t_port *priv, int size)
{
int room;
@@ -889,9 +873,10 @@ static unsigned int handle_tx(struct eg20t_port *priv)
fifo_size = max(priv->fifo_size, 1);
tx_empty = 1;
- if (pop_tx_x(priv, xmit->buf)) {
- pch_uart_hal_write(priv, xmit->buf, 1);
+ if (port->x_char) {
+ pch_uart_hal_write(priv, &port->x_char, 1);
port->icount.tx++;
+ port->x_char = 0;
tx_empty = 0;
fifo_size--;
}
@@ -948,9 +933,11 @@ static unsigned int dma_handle_tx(struct eg20t_port *priv)
}
fifo_size = max(priv->fifo_size, 1);
- if (pop_tx_x(priv, xmit->buf)) {
- pch_uart_hal_write(priv, xmit->buf, 1);
+
+ if (port->x_char) {
+ pch_uart_hal_write(priv, &port->x_char, 1);
port->icount.tx++;
+ port->x_char = 0;
fifo_size--;
}
--
2.36.0
It's horrid and if we inline it into callers, we can get rid of a lot of
sugar around.
So:
* x_char handling becomes a single iowrite8.
* xmit->buf handling is a single loop simply writing characters one by
one directly from the buf instead of complex cnt_to_end computations.
Until the buf is empty or fifo size is reached.
Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/serial/pch_uart.c | 46 ++++++++++-------------------------
1 file changed, 13 insertions(+), 33 deletions(-)
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 6cb631487383..4fcb6c144b54 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -550,18 +550,6 @@ static u8 pch_uart_hal_get_modem(struct eg20t_port *priv)
return (u8)msr;
}
-static void pch_uart_hal_write(struct eg20t_port *priv,
- const unsigned char *buf, int tx_size)
-{
- int i;
- unsigned int thr;
-
- for (i = 0; i < tx_size;) {
- thr = buf[i++];
- iowrite8(thr, priv->membase + PCH_UART_THR);
- }
-}
-
static int pch_uart_hal_read(struct eg20t_port *priv, unsigned char *buf,
int rx_size)
{
@@ -769,23 +757,21 @@ static void pch_dma_tx_complete(void *arg)
pch_uart_hal_enable_interrupt(priv, PCH_UART_HAL_TX_INT);
}
-static int pop_tx(struct eg20t_port *priv, int size)
+static bool pop_tx(struct eg20t_port *priv, unsigned int size)
{
- int count = 0;
+ unsigned int count = 0;
struct uart_port *port = &priv->port;
struct circ_buf *xmit = &port->state->xmit;
- if (uart_tx_stopped(port) || uart_circ_empty(xmit))
+ if (uart_tx_stopped(port))
goto pop_tx_end;
- do {
- int cnt_to_end =
- CIRC_CNT_TO_END(xmit->head, xmit->tail, UART_XMIT_SIZE);
- int sz = min(size - count, cnt_to_end);
- pch_uart_hal_write(priv, &xmit->buf[xmit->tail], sz);
- xmit->tail = (xmit->tail + sz) & (UART_XMIT_SIZE - 1);
- count += sz;
- } while (!uart_circ_empty(xmit) && count < size);
+ while (!uart_circ_empty(xmit) && count < size) {
+ iowrite8(xmit->buf[xmit->tail], priv->membase + PCH_UART_THR);
+ xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
+ port->icount.tx++;
+ count++;
+ }
pop_tx_end:
dev_dbg(priv->port.dev, "%d characters. Remained %d characters.(%lu)\n",
@@ -859,7 +845,6 @@ static unsigned int handle_tx(struct eg20t_port *priv)
struct uart_port *port = &priv->port;
struct circ_buf *xmit = &port->state->xmit;
int fifo_size;
- int tx_size;
int size;
int tx_empty;
@@ -874,7 +859,7 @@ static unsigned int handle_tx(struct eg20t_port *priv)
fifo_size = max(priv->fifo_size, 1);
tx_empty = 1;
if (port->x_char) {
- pch_uart_hal_write(priv, &port->x_char, 1);
+ iowrite8(port->x_char, priv->membase + PCH_UART_THR);
port->icount.tx++;
port->x_char = 0;
tx_empty = 0;
@@ -884,13 +869,8 @@ static unsigned int handle_tx(struct eg20t_port *priv)
size = min(xmit->head - xmit->tail, fifo_size);
if (size < 0)
size = fifo_size;
- if (size) {
- tx_size = pop_tx(priv, size);
- if (tx_size > 0) {
- port->icount.tx += tx_size;
- tx_empty = 0;
- }
- }
+ if (size && pop_tx(priv, size))
+ tx_empty = 0;
priv->tx_empty = tx_empty;
@@ -935,7 +915,7 @@ static unsigned int dma_handle_tx(struct eg20t_port *priv)
fifo_size = max(priv->fifo_size, 1);
if (port->x_char) {
- pch_uart_hal_write(priv, &port->x_char, 1);
+ iowrite8(port->x_char, priv->membase + PCH_UART_THR);
port->icount.tx++;
port->x_char = 0;
fifo_size--;
--
2.36.0
One is in handle_tx() (as "min(xmit->head - xmit->tail, fifo_size))",
another one in pop_tx() (as uart_circ_empty(xmit)). So keep only the
latter.
This makes the code simpler and size variable is not needed now.
Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/serial/pch_uart.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index a90bdff60908..ae1d6b641253 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -839,9 +839,7 @@ static int dma_handle_rx(struct eg20t_port *priv)
static unsigned int handle_tx(struct eg20t_port *priv)
{
struct uart_port *port = &priv->port;
- struct circ_buf *xmit = &port->state->xmit;
int fifo_size;
- int size;
int tx_empty;
if (!priv->start_tx) {
@@ -862,10 +860,7 @@ static unsigned int handle_tx(struct eg20t_port *priv)
fifo_size--;
}
- size = min(xmit->head - xmit->tail, fifo_size);
- if (size < 0)
- size = fifo_size;
- if (size && pop_tx(priv, size))
+ if (fifo_size && pop_tx(priv, fifo_size))
tx_empty = 0;
priv->tx_empty = tx_empty;
--
2.36.0
1) take uart_tx_stopped into account every loop (the same as other uart
drivers)
2) no need for 'count' variable, operate on 'size' directly
This allows inlining this into handle_tx() nicely in the next patch.
Signed-off-by: Jiri Slaby <[email protected]>
---
drivers/tty/serial/pch_uart.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index ae1d6b641253..e1eadf519089 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -759,21 +759,19 @@ static void pch_dma_tx_complete(void *arg)
static bool pop_tx(struct eg20t_port *priv, unsigned int size)
{
- unsigned int count = 0;
struct uart_port *port = &priv->port;
struct circ_buf *xmit = &port->state->xmit;
+ bool ret = false;
- if (uart_tx_stopped(port))
- return false;
-
- while (!uart_circ_empty(xmit) && count < size) {
+ while (!uart_tx_stopped(port) && !uart_circ_empty(xmit) && size) {
iowrite8(xmit->buf[xmit->tail], priv->membase + PCH_UART_THR);
xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE - 1);
port->icount.tx++;
- count++;
+ size--;
+ ret = true;
}
- return count;
+ return ret;
}
static int handle_rx_to(struct eg20t_port *priv)
--
2.36.0