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

[Chris Patterson <cjp256@gmail.com>]

On Wed, Aug 17, 2016 at 10:29 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>> On 17.08.16 at 16:02, <cjp256@gmail.com> wrote:
>> From: Chris Patterson <pattersonc@ainfosec.com>
>>
>> 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
>

ACK, will fix these in v2.  Thank you for the review.

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