* Paolo Bonzini (pbonz...@redhat.com) wrote:
> - 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.
>
> Signed-off-by: Paolo Bonzini <pbonz...@redhat.com>
> ---
> 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 8c42d03..4bce268 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));
That's undoing dsl...@verizon.com's
dffacd46 - Fix emptyness checking
See, http://permalink.gmane.org/gmane.comp.emulators.qemu/262412
I don't think your assumptions are safe because of that qemu_chr_fe_add_watch.
Dave
> 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
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
