Re: [Xen-devel] [PATCH] ns16550: mask transmit holding register empty interrupt when tx is stopped

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 17.08.16 at 16:02, <cjp256@xxxxxxxxx> wrote:
> From: Chris Patterson <pattersonc@xxxxxxxxxxxx>
> 
> The uart generates an interrupt whenever the transmit holding register is
> empty and UART_IER_ETHREI is set in UART_IER.  Currently, Xen's ns16550
> driver does not currently mask this interrupt when transmit is stopped,
> unlike other platforms such as Linux [1].
> 
> Toggle UART_IER_ETHREI flag in the UART_IER according to the state dictated
> by stop_tx and start_tx hooks.
> 
> On the Tegra platform (forthcoming series), the reset via reading IIR does not
> prevent re-assertion of THRE.  This causes Xen to hang in the interrupt
> handler's while loop whenever there is no data to transmit.  This behavior 
> (bug?)
> is addressed by utilizing the start & stop tx hooks.

Looks mostly okay from a technical pov, but there are a few minor
(mostly style) issues.

> [1] 
> http://lxr.free-electrons.com/source/drivers/tty/serial/8250/8250_port.c?v=4.7#L1518

I'd appreciate for such references to be to the canonical (i.e. Linus'es)
tree.

> @@ -813,6 +813,26 @@ static int __init ns16550_irq(struct serial_port *port)
>      return ((uart->irq > 0) ? uart->irq : -1);
>  }
>  
> +static void ns16550_start_tx(struct serial_port *port)
> +{
> +    struct ns16550 *uart = port->uart;
> +    unsigned int ier = ns_read_reg(uart, UART_IER);

Please use u8 or unsigned char here, as is done elsewhere in the file.

> +    /* unmask transmit holding register empty interrupt if currently masked 
> */

Comment style: Upper case at the beginning; the full stop at the
end has become optional recently.

> +    if (!(ier & UART_IER_ETHREI))

Blanks missing inside the parentheses.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel