From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53445) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xz8KT-0003Hg-DO for qemu-devel@nongnu.org; Thu, 11 Dec 2014 13:19:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xz8KN-0003dc-Cq for qemu-devel@nongnu.org; Thu, 11 Dec 2014 13:19:09 -0500 Sender: Paolo Bonzini From: Paolo Bonzini Date: Thu, 11 Dec 2014 19:18:49 +0100 Message-Id: <1418321931-12648-2-git-send-email-pbonzini@redhat.com> In-Reply-To: <1418321931-12648-1-git-send-email-pbonzini@redhat.com> References: <1418321931-12648-1-git-send-email-pbonzini@redhat.com> Subject: [Qemu-devel] [PATCH 1/3] serial: clean up THRE/TEMT handling List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: imammedo@redhat.com, qemu-stable@nongnu.org, andrey@xdel.ru, dgilbert@redhat.com, batuzovk@ispras.ru - assert THRE cleared and FIFO not empty (if enabled) before sending a character. Also assert TEMT cleared, since it is the combination of THRE && transmitter shift register empty. - raise THRI immediately after setting THRE - check THRE to see if another character has to be sent, which makes the assertions more obvious and also means TEMT has to be set as soon as the loop ends - clear TEMT together with THRE even in the non-FIFO case There are certainly a couple bugfixes in here, but nothing that squashes known bugs. Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini --- hw/char/serial.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/hw/char/serial.c b/hw/char/serial.c index ebcacdc..4cd139f 100644 --- a/hw/char/serial.c +++ b/hw/char/serial.c @@ -224,21 +224,23 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque) SerialState *s = opaque; do { + assert(!(s->lsr & UART_LSR_TEMT)); + assert(!(s->lsr & UART_LSR_THRE)); + if (s->tsr_retry <= 0) { if (s->fcr & UART_FCR_FE) { - if (fifo8_is_empty(&s->xmit_fifo)) { - return FALSE; - } + assert(!fifo8_is_empty(&s->xmit_fifo)); s->tsr = fifo8_pop(&s->xmit_fifo); if (!s->xmit_fifo.num) { s->lsr |= UART_LSR_THRE; } - } else if ((s->lsr & UART_LSR_THRE)) { - return FALSE; } else { s->tsr = s->thr; s->lsr |= UART_LSR_THRE; - s->lsr &= ~UART_LSR_TEMT; + } + if ((s->lsr & UART_LSR_THRE) && !s->thr_ipending) { + s->thr_ipending = 1; + serial_update_irq(s); } } @@ -256,17 +258,13 @@ static gboolean serial_xmit(GIOChannel *chan, GIOCondition cond, void *opaque) } else { s->tsr_retry = 0; } + /* Transmit another byte if it is already available. It is only possible when FIFO is enabled and not empty. */ - } while ((s->fcr & UART_FCR_FE) && !fifo8_is_empty(&s->xmit_fifo)); + } while (!(s->lsr & UART_LSR_THRE)); s->last_xmit_ts = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL); - - if (s->lsr & UART_LSR_THRE) { - s->lsr |= UART_LSR_TEMT; - s->thr_ipending = 1; - serial_update_irq(s); - } + s->lsr |= UART_LSR_TEMT; return FALSE; } @@ -323,10 +321,10 @@ static void serial_ioport_write(void *opaque, hwaddr addr, uint64_t val, fifo8_pop(&s->xmit_fifo); } fifo8_push(&s->xmit_fifo, s->thr); - s->lsr &= ~UART_LSR_TEMT; } s->thr_ipending = 0; s->lsr &= ~UART_LSR_THRE; + s->lsr &= ~UART_LSR_TEMT; serial_update_irq(s); if (s->tsr_retry <= 0) { serial_xmit(NULL, G_IO_OUT, s); -- 1.8.3.1