From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=40026 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1OHDuW-0005zE-JF
	for qemu-devel@nongnu.org; Wed, 26 May 2010 06:32:29 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <jan.kiszka@siemens.com>) id 1OHDuV-00008i-I6
	for qemu-devel@nongnu.org; Wed, 26 May 2010 06:32:28 -0400
Received: from david.siemens.de ([192.35.17.14]:24333)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <jan.kiszka@siemens.com>) id 1OHDuV-00005I-7p
	for qemu-devel@nongnu.org; Wed, 26 May 2010 06:32:27 -0400
Message-ID: <4BFCF8AF.8030300@siemens.com>
Date: Wed, 26 May 2010 12:32:15 +0200
From: Jan Kiszka <jan.kiszka@siemens.com>
MIME-Version: 1.0
References: <201005261207.31310.frank.mehnert@sun.com>
In-Reply-To: <201005261207.31310.frank.mehnert@sun.com>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: hw/serial.c: Xmit fifo never used
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Frank Mehnert <Frank.Mehnert@Sun.COM>
Cc: qemu-devel@nongnu.org, Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Frank Mehnert wrote:
> Hi,
> 
> the xmit fifo of the serial device is never used. If qemu_chr_write()
> fails (interface currently not able to send characters) then the
> transmit_timer should be engaged to try to send the current character
> from the fifo again after some time. The code is
> 
>     } else if (qemu_chr_write(s->chr, &s->tsr, 1) != 1) {
>         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;
>         }
>         ...
>     }
> 
> The problem is that this path is never used as tsr_retry is never > 0
> initially. So if qemu_chr_write() fails, we never try again but drop
> the character.
> 
> I assume the correct condition would be '>= 0', that is
> 
>         ...
>         if ((s->tsr_retry >= 0) && (s->tsr_retry <= MAX_XMIT_RETRY)) {
>             s->tsr_retry++;
>         ...

Makes sense, patch welcome.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux