The hardware has a 128 byte RX FIFO buffer for each independent UART.
Previously, the code was always reading that byte-by-byte via
independent SPI transactions and the associated overhead. In practice,
this led to up to eight bytes over SPI for just one byte in the UART's
RX FIFO:
- reading the global IRQ register (two bytes, one for command, the other
for data)
- reading one UART's ISR (again two bytes)
- reading the byte count (two bytes yet again)
- finally, reading one byte of the FIFO via another two-byte transaction
We cannot always use a batched read. If the TTY is set to intercept
break conditions or report framing or parity errors, then it is required
to check the Line Status Register (LSR) for each byte which is read from
the RX FIFO. The documentation does not show a way of doing that in a
single SPI transaction; registers 0x00 and 0x04 are separate.
In my testing, this is no silver bullet. I was feeding 2MB of random
data over four daisy-chaned UARTs of MAX14830, and this is the
distribution that I was getting:
- R <= 1: 7437322
- R <= 2: 162093
- R <= 4: 4093
- R <= 8: 4196
- R <= 16: 645
- R <= 32: 165
- R <= 64: 58
- R <= 128: 0
For a reference, batching the write operations works much better:
- W <= 1: 2664
- W <= 2: 1305
- W <= 4: 627
- W <= 8: 371
- W <= 16: 121
- W <= 32: 68
- W <= 64: 33
- W <= 128: 63139
That's probably because this HW/SW combination (Clearfog Base, Armada
388) is probably "good enough" to react to the chip's IRQ "fast enough"
most of the time. Still, I was getting RX overruns every now and then.
In future, I plan to improve this by letting the RX FIFO be filled a
little more (the chip has support for that and also for a "stale
timeout" to prevent additional starvation).
Signed-off-by: Jan Kundrát <jan.kundrat@xxxxxxxxx>
---
drivers/tty/serial/max310x.c | 126 +++++++++++++++++++++++++++++--------------
1 file changed, 86 insertions(+), 40 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 2a2c4ea16306..dc0e1ad351f9 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -609,57 +609,103 @@ static void max310x_batch_write(struct uart_port *port, u8 *txbuf, unsigned int
spi_sync_transfer(to_spi_device(port->dev), xfer, ARRAY_SIZE(xfer));
}
-static void max310x_handle_rx(struct uart_port *port, unsigned int rxlen)
+static void max310x_batch_read(struct uart_port *port, u8 *rxbuf, unsigned int len)
{
- unsigned int sts, ch, flag;
+ u8 header[] = { port->iobase + MAX310X_RHR_REG };
+ struct spi_transfer xfer[] = {
+ {
+ .tx_buf = &header,
+ .len = sizeof(header),
+ }, {
+ .rx_buf = rxbuf,
+ .len = len,
+ }
+ };
+ spi_sync_transfer(to_spi_device(port->dev), xfer, ARRAY_SIZE(xfer));
+}
- if (unlikely(rxlen >= port->fifosize)) {
- dev_warn_ratelimited(port->dev, "Possible RX FIFO overrun\n");
- port->icount.buf_overrun++;
- /* Ensure sanity of RX level */
- rxlen = port->fifosize;
- }
+static void max310x_handle_rx(struct uart_port *port, unsigned int rxlen)
+{
+ unsigned int sts, ch, flag, i;
+ u8 buf[MAX310X_FIFO_SIZE];
+
+ if (port->read_status_mask == MAX310X_LSR_RXOVR_BIT) {
+ /* We are just reading, happily ignoring any error conditions.
+ * Break condition, parity checking, framing errors -- they
+ * are all ignored. That means that we can do a batch-read.
+ *
+ * There is a small opportunity for race if the RX FIFO
+ * overruns while we're reading the buffer; the datasheets says
+ * that the LSR register applies to the "current" character.
+ * That's also the reason why we cannot do batched reads when
+ * asked to check the individual statuses.
+ * */
- while (rxlen--) {
- ch = max310x_port_read(port, MAX310X_RHR_REG);
sts = max310x_port_read(port, MAX310X_LSR_IRQSTS_REG);
+ max310x_batch_read(port, buf, rxlen);
- sts &= MAX310X_LSR_RXPAR_BIT | MAX310X_LSR_FRERR_BIT |
- MAX310X_LSR_RXOVR_BIT | MAX310X_LSR_RXBRK_BIT;
-
- port->icount.rx++;
+ port->icount.rx += rxlen;
flag = TTY_NORMAL;
+ sts &= port->read_status_mask;
- if (unlikely(sts)) {
- if (sts & MAX310X_LSR_RXBRK_BIT) {
- port->icount.brk++;
- if (uart_handle_break(port))
- continue;
- } else if (sts & MAX310X_LSR_RXPAR_BIT)
- port->icount.parity++;
- else if (sts & MAX310X_LSR_FRERR_BIT)
- port->icount.frame++;
- else if (sts & MAX310X_LSR_RXOVR_BIT)
- port->icount.overrun++;
-
- sts &= port->read_status_mask;
- if (sts & MAX310X_LSR_RXBRK_BIT)
- flag = TTY_BREAK;
- else if (sts & MAX310X_LSR_RXPAR_BIT)
- flag = TTY_PARITY;
- else if (sts & MAX310X_LSR_FRERR_BIT)
- flag = TTY_FRAME;
- else if (sts & MAX310X_LSR_RXOVR_BIT)
- flag = TTY_OVERRUN;
+ if (sts & MAX310X_LSR_RXOVR_BIT) {
+ dev_warn_ratelimited(port->dev, "Hardware RX FIFO overrun\n");
+ port->icount.overrun++;
}
- if (uart_handle_sysrq_char(port, ch))
- continue;
+ for (i = 0; i < rxlen; ++i) {
+ uart_insert_char(port, sts, MAX310X_LSR_RXOVR_BIT, buf[i], flag);
+ }
- if (sts & port->ignore_status_mask)
- continue;
+ } else {
+ if (unlikely(rxlen >= port->fifosize)) {
+ dev_warn_ratelimited(port->dev, "Possible RX FIFO overrun\n");
+ port->icount.buf_overrun++;
+ /* Ensure sanity of RX level */
+ rxlen = port->fifosize;
+ }
- uart_insert_char(port, sts, MAX310X_LSR_RXOVR_BIT, ch, flag);
+ while (rxlen--) {
+ ch = max310x_port_read(port, MAX310X_RHR_REG);
+ sts = max310x_port_read(port, MAX310X_LSR_IRQSTS_REG);
+
+ sts &= MAX310X_LSR_RXPAR_BIT | MAX310X_LSR_FRERR_BIT |
+ MAX310X_LSR_RXOVR_BIT | MAX310X_LSR_RXBRK_BIT;
+
+ port->icount.rx++;
+ flag = TTY_NORMAL;
+
+ if (unlikely(sts)) {
+ if (sts & MAX310X_LSR_RXBRK_BIT) {
+ port->icount.brk++;
+ if (uart_handle_break(port))
+ continue;
+ } else if (sts & MAX310X_LSR_RXPAR_BIT)
+ port->icount.parity++;
+ else if (sts & MAX310X_LSR_FRERR_BIT)
+ port->icount.frame++;
+ else if (sts & MAX310X_LSR_RXOVR_BIT)
+ port->icount.overrun++;
+
+ sts &= port->read_status_mask;
+ if (sts & MAX310X_LSR_RXBRK_BIT)
+ flag = TTY_BREAK;
+ else if (sts & MAX310X_LSR_RXPAR_BIT)
+ flag = TTY_PARITY;
+ else if (sts & MAX310X_LSR_FRERR_BIT)
+ flag = TTY_FRAME;
+ else if (sts & MAX310X_LSR_RXOVR_BIT)
+ flag = TTY_OVERRUN;
+ }
+
+ if (uart_handle_sysrq_char(port, ch))
+ continue;
+
+ if (sts & port->ignore_status_mask)
+ continue;
+
+ uart_insert_char(port, sts, MAX310X_LSR_RXOVR_BIT, ch, flag);
+ }
}
tty_flip_buffer_push(&port->state->port);
--
2.14.3
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
