On Fri, 16 Feb 2024, Leonardo Bras wrote:
> For every TX byte an IRQ is requested.
> On threaded IRQs, the handler calls serial8250_tx_chars can send multiple
> bytes, limited to it's queue size (tx_loadsz).
Perhaps I'm missing something here but I don't understand what this tries
to say.
- 8250 driver gets TX empty IRQ
- We write x bytes to FIFO
- UART blasts those bits to wire, eventually emptying FIFO
- We get the next TX empty IRQ
What in this makes "for every TX byte an IRQ is requested" true? There's
one IRQ only for every x bytes TX'ed as far as I can tell!?!
--
i.
> When this happens, the handler return IRQ_HANDLED with reduces the
> unhandled IRQ counter only by 1, even though many requests have been
> handled at once.
>
> This causes the unhandled IRQ counter to go up until it reaches the maximum
> and causes the registered IRQ to be disabled, thus breaking the serial
> console.
>
> Make use of the newly introduced IRQ_HANDLED_MANY interface to return the
> number of requests handled, so the unhandled IRQ counter can get decreased
> accordingly.
>
> Signed-off-by: Leonardo Bras <leobras@xxxxxxxxxx>
> ---
> include/linux/serial_8250.h | 2 +-
> drivers/tty/serial/8250/8250_core.c | 13 ++++++++-----
> drivers/tty/serial/8250/8250_port.c | 16 ++++++++++------
> 3 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/serial_8250.h b/include/linux/serial_8250.h
> index ec46e3b49ee99..c9d4271b71d70 100644
> --- a/include/linux/serial_8250.h
> +++ b/include/linux/serial_8250.h
> @@ -200,7 +200,7 @@ int fsl8250_handle_irq(struct uart_port *port);
> int serial8250_handle_irq(struct uart_port *port, unsigned int iir);
> u16 serial8250_rx_chars(struct uart_8250_port *up, u16 lsr);
> void serial8250_read_char(struct uart_8250_port *up, u16 lsr);
> -void serial8250_tx_chars(struct uart_8250_port *up);
> +int serial8250_tx_chars(struct uart_8250_port *up);
> unsigned int serial8250_modem_status(struct uart_8250_port *up);
> void serial8250_init_port(struct uart_8250_port *up);
> void serial8250_set_defaults(struct uart_8250_port *up);
> diff --git a/drivers/tty/serial/8250/8250_core.c b/drivers/tty/serial/8250/8250_core.c
> index ae637155fe7cd..2fab9102eec45 100644
> --- a/drivers/tty/serial/8250/8250_core.c
> +++ b/drivers/tty/serial/8250/8250_core.c
> @@ -110,7 +110,7 @@ static irqreturn_t serial8250_interrupt(int irq, void *dev_id)
> {
> struct irq_info *i = dev_id;
> struct list_head *l, *end = NULL;
> - int pass_counter = 0, handled = 0;
> + int pass_counter = 0, handled_total = 0;
>
> pr_debug("%s(%d): start\n", __func__, irq);
>
> @@ -120,15 +120,18 @@ static irqreturn_t serial8250_interrupt(int irq, void *dev_id)
> do {
> struct uart_8250_port *up;
> struct uart_port *port;
> + int handled;
>
> up = list_entry(l, struct uart_8250_port, list);
> port = &up->port;
>
> - if (port->handle_irq(port)) {
> - handled = 1;
> + handled = port->handle_irq(port);
> + if (handled) {
> + handled_total += handled;
> end = NULL;
> - } else if (end == NULL)
> + } else if (end == NULL) {
> end = l;
> + }
>
> l = l->next;
>
> @@ -140,7 +143,7 @@ static irqreturn_t serial8250_interrupt(int irq, void *dev_id)
>
> pr_debug("%s(%d): end\n", __func__, irq);
>
> - return IRQ_RETVAL(handled);
> + return IRQ_RETVAL_MANY(handled_total);
> }
>
> /*
> diff --git a/drivers/tty/serial/8250/8250_port.c b/drivers/tty/serial/8250/8250_port.c
> index f799c34f1603c..74d53507a73d4 100644
> --- a/drivers/tty/serial/8250/8250_port.c
> +++ b/drivers/tty/serial/8250/8250_port.c
> @@ -1802,7 +1802,7 @@ u16 serial8250_rx_chars(struct uart_8250_port *up, u16 lsr)
> }
> EXPORT_SYMBOL_GPL(serial8250_rx_chars);
>
> -void serial8250_tx_chars(struct uart_8250_port *up)
> +int serial8250_tx_chars(struct uart_8250_port *up)
> {
> struct uart_port *port = &up->port;
> struct circ_buf *xmit = &port->state->xmit;
> @@ -1810,15 +1810,15 @@ void serial8250_tx_chars(struct uart_8250_port *up)
>
> if (port->x_char) {
> uart_xchar_out(port, UART_TX);
> - return;
> + return 0;
> }
> if (uart_tx_stopped(port)) {
> serial8250_stop_tx(port);
> - return;
> + return 0;
> }
> if (uart_circ_empty(xmit)) {
> __stop_tx(up);
> - return;
> + return 0;
> }
>
> count = up->tx_loadsz;
> @@ -1858,6 +1858,9 @@ void serial8250_tx_chars(struct uart_8250_port *up)
> */
> if (uart_circ_empty(xmit) && !(up->capabilities & UART_CAP_RPM))
> __stop_tx(up);
> +
> + /* Return number of chars sent */
> + return up->tx_loadsz - count;
> }
> EXPORT_SYMBOL_GPL(serial8250_tx_chars);
>
> @@ -1923,6 +1926,7 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
> bool skip_rx = false;
> unsigned long flags;
> u16 status;
> + int handled = 0;
>
> if (iir & UART_IIR_NO_INT)
> return 0;
> @@ -1956,14 +1960,14 @@ int serial8250_handle_irq(struct uart_port *port, unsigned int iir)
> serial8250_modem_status(up);
> if ((status & UART_LSR_THRE) && (up->ier & UART_IER_THRI)) {
> if (!up->dma || up->dma->tx_err)
> - serial8250_tx_chars(up);
> + handled = serial8250_tx_chars(up);
> else if (!up->dma->tx_running)
> __stop_tx(up);
> }
>
> uart_unlock_and_check_sysrq_irqrestore(port, flags);
>
> - return 1;
> + return handled ? : 1;
> }
> EXPORT_SYMBOL_GPL(serial8250_handle_irq);
>
>
