From patchwork Mon Apr 2 14:39:38 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Anthony Liguori X-Patchwork-Id: 150169 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 3FCE1B6EEC for ; Tue, 3 Apr 2012 00:40:23 +1000 (EST) Received: from localhost ([::1]:57683 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SEiQb-0004kP-Sy for incoming@patchwork.ozlabs.org; Mon, 02 Apr 2012 10:40:17 -0400 Received: from eggs.gnu.org ([208.118.235.92]:58913) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SEiQD-0004Yz-86 for qemu-devel@nongnu.org; Mon, 02 Apr 2012 10:39:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SEiQB-0003zf-Ep for qemu-devel@nongnu.org; Mon, 02 Apr 2012 10:39:52 -0400 Received: from cpe-70-123-132-139.austin.res.rr.com ([70.123.132.139]:34426 helo=localhost6.localdomain6) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SEiQB-0003xx-7i for qemu-devel@nongnu.org; Mon, 02 Apr 2012 10:39:51 -0400 Received: from localhost6.localdomain6 (localhost.localdomain [127.0.0.1]) by localhost6.localdomain6 (8.14.4/8.14.4/Debian-2ubuntu2) with ESMTP id q32Edh6H007679; Mon, 2 Apr 2012 09:39:43 -0500 Received: (from anthony@localhost) by localhost6.localdomain6 (8.14.4/8.14.4/Submit) id q32EdfZb007678; Mon, 2 Apr 2012 09:39:41 -0500 From: Anthony Liguori To: qemu-devel@nongnu.org Date: Mon, 2 Apr 2012 09:39:38 -0500 Message-Id: <1333377579-7513-2-git-send-email-aliguori@us.ibm.com> X-Mailer: git-send-email 1.7.5.4 In-Reply-To: <1333377579-7513-1-git-send-email-aliguori@us.ibm.com> References: <1333377579-7513-1-git-send-email-aliguori@us.ibm.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 70.123.132.139 Cc: Anthony Liguori , Stefano Stabellini Subject: [Qemu-devel] [PATCH 1/2] serial: fix retry logic X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org I'm not sure if the retry logic has ever worked when not using FIFO mode. I found this while writing a test case although code inspection confirms it is definitely broken. The TSR retry logic will never actually happen because it is guarded by an 'if (s->tsr_rety > 0)' but this is the only place that can ever make the variable greater than zero. That effectively makes the retry logic an 'if (0)'. I believe this is a typo and the intention was >= 0. Once this is fixed though, I see double transmits with my test case. This is because in the non FIFO case, serial_xmit may get invoked while LSR.THRE is still high because the character was processed but the retransmit timer was still active. We can handle this by simply checking for LSR.THRE and returning early. It's possible that the FIFO paths also need some attention. Cc: Stefano Stabellini Signed-off-by: Anthony Liguori --- hw/serial.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/hw/serial.c b/hw/serial.c index c0ee55d..b499bca 100644 --- a/hw/serial.c +++ b/hw/serial.c @@ -327,6 +327,8 @@ static void serial_xmit(void *opaque) s->tsr = fifo_get(s,XMIT_FIFO); if (!s->xmit_fifo.count) s->lsr |= UART_LSR_THRE; + } else if ((s->lsr & UART_LSR_THRE)) { + return; } else { s->tsr = s->thr; s->lsr |= UART_LSR_THRE; @@ -337,7 +339,7 @@ static void serial_xmit(void *opaque) /* in loopback mode, say that we just received a char */ serial_receive1(s, &s->tsr, 1); } else if (qemu_chr_fe_write(s->chr, &s->tsr, 1) != 1) { - if ((s->tsr_retry > 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) { + if ((s->tsr_retry >= 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) { s->tsr_retry++; qemu_mod_timer(s->transmit_timer, new_xmit_ts + s->char_transmit_time); return;