serial: max310x: Use batched reads when reasonably safe

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@cesnet.cz>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Jan Kundrát 2017-12-14 16:02:54 +01:00 committed by Greg Kroah-Hartman
parent d584b65c0d
commit 2b4bac48c1

View File

@ -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_batch_read(struct uart_port *port, u8 *rxbuf, unsigned int len)
{
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));
}
static void max310x_handle_rx(struct uart_port *port, unsigned int rxlen)
{
unsigned int sts, ch, flag;
unsigned int sts, ch, flag, i;
u8 buf[MAX310X_FIFO_SIZE];
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;
}
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);