[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 2/4] serial: clean up THRE/TEMT handling
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v3 2/4] serial: clean up THRE/TEMT handling |
Date: |
Mon, 15 Dec 2014 11:40:19 +0000 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
* Paolo Bonzini (address@hidden) 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 <address@hidden>
> ---
> 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 address@hidden'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 / address@hidden / Manchester, UK
- Re: [Qemu-devel] [PATCH v3 1/4] serial: reset thri_pending on IER writes with THRI=0, (continued)
[Qemu-devel] [PATCH v3 4/4] serial: only resample THR interrupt on rising edge of IER.THRI, Paolo Bonzini, 2014/12/12
[Qemu-devel] [PATCH v3 3/4] serial: update LSR on enabling/disabling FIFOs, Paolo Bonzini, 2014/12/12
[Qemu-devel] [PATCH v3 2/4] serial: clean up THRE/TEMT handling, Paolo Bonzini, 2014/12/12
- Re: [Qemu-devel] [PATCH v3 2/4] serial: clean up THRE/TEMT handling,
Dr. David Alan Gilbert <=