[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] upgrading emulated UART to 16550A
From: |
Anthony Liguori |
Subject: |
Re: [Qemu-devel] [PATCH] upgrading emulated UART to 16550A |
Date: |
Thu, 07 Aug 2008 13:58:43 -0500 |
User-agent: |
Thunderbird 2.0.0.14 (X11/20080501) |
Stefano Stabellini wrote:
This is an improved version of the UART 16550A emulation patch.
The changes compared to previous version are:
- no token bucket anymore;
- fixed a small bug handling IRQs; this was the problem that prevented
kgdb to work over the serial (thanks to Jason Wessel for the help
spotting and reproducing this bug).
Signed-off-by: Stefano Stabellini <address@hidden>
---
diff --git a/hw/serial.c b/hw/serial.c
index ffd6ac9..5aab00e 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -1,7 +1,7 @@
/*
- * QEMU 16450 UART emulation
+ * QEMU 16550A UART emulation
*
- * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2003-2008 Fabrice Bellard
I think the idea was to add a new copyright entry, not to modify
Fabrice's copyright.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to
deal
@@ -21,6 +21,10 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/
+
+#include <termios.h>
+#include <sys/ioctl.h>
This doesn't look Windows friendly.
@@ -100,55 +131,105 @@ struct SerialState {
target_phys_addr_t base;
int it_shift;
int baudbase;
- QEMUTimer *tx_timer;
- int tx_burst;
+ int tsr_retry;
+
+ uint64_t last_xmit_ts; /* Time when the last byte was
successfully sent out of the tsr */
+ SerialFIFO recv_fifo;
+ SerialFIFO xmit_fifo;
+
+ struct QEMUTimer *fifo_timeout_timer;
+ int timeout_ipending; /* timeout interrupt pending state
*/
+ struct QEMUTimer *transmit_timer;
+
+
+ uint64_t char_transmit_time; /* time to transmit a char in
ticks*/
+ int poll_msl;
+
+ struct QEMUTimer *modem_status_poll;
};
+static void serial_receive1(void *opaque, const uint8_t *buf, int size);
+
+static void fifo_clear(SerialState *s, int fifo) {
minor nit, but the '{' should be on a new line.
+ SerialFIFO *f = ( fifo ) ? &s->recv_fifo : &s->xmit_fifo;
+ memset(f->data, 0, UART_FIFO_LENGTH);
+ f->count = 0;
+ f->head = 0;
+ f->tail = 0;
+}
+
+static int fifo_put(SerialState *s, int fifo, uint8_t chr) {
+ SerialFIFO *f = ( fifo ) ? &s->recv_fifo : &s->xmit_fifo;
+
+ f->data[f->head++] = chr;
+
+ if (f->head == UART_FIFO_LENGTH)
+ f->head = 0;
+ f->count++;
+
+ return 1;
+}
+
+uint8_t fifo_get(SerialState *s, int fifo) {
Any reason for this not to be static?
-static void serial_tx_done(void *opaque)
-{
- SerialState *s = opaque;
- if (s->tx_burst < 0) {
- uint16_t divider;
+ if ( ( s->ier & UART_IER_RLSI ) && (s->lsr & UART_LSR_INT_ANY ) ) {
+ tmp_iir = UART_IIR_RLSI;
+ } else if ( s->timeout_ipending ) {
+ tmp_iir = UART_IIR_CTI;
+ } else if ( ( s->ier & UART_IER_RDI ) && (s->lsr & UART_LSR_DR ) ) {
+ if ( !(s->fcr & UART_FCR_FE) ) {
+ tmp_iir = UART_IIR_RDI;
+ } else if ( s->recv_fifo.count >= s->recv_fifo.itl ) {
+ tmp_iir = UART_IIR_RDI;
+ }
+ } else if ( (s->ier & UART_IER_THRI) && s->thr_ipending ) {
+ tmp_iir = UART_IIR_THRI;
+ } else if ( (s->ier & UART_IER_MSI) && (s->msr & UART_MSR_ANY_DELTA) ) {
+ tmp_iir = UART_IIR_MSI;
+ }
- if (s->divider)
- divider = s->divider;
- else
- divider = 1;
+ s->iir = tmp_iir | ( s->iir & 0xF0 );
- /* We assume 10 bits/char, OK for this purpose. */
- s->tx_burst = THROTTLE_TX_INTERVAL * 1000 /
- (1000000 * 10 / (s->baudbase / divider));
+ if ( tmp_iir != UART_IIR_NO_INT ) {
+ qemu_irq_raise(s->irq);
+ } else {
+ qemu_irq_lower(s->irq);
More nits, lose the whitespace in the conditions. For instance:
} else if ( ( s->ier & UART_IER_RDI ) => } else if ((s->ier &
UART_IER_RDI).
The rest of the file uses the later style so it's a little weird to have
portions of the code be different.
}
- s->thr_ipending = 1;
- s->lsr |= UART_LSR_THRE;
- s->lsr |= UART_LSR_TEMT;
- serial_update_irq(s);
}
static void serial_update_parameters(SerialState *s)
{
- int speed, parity, data_bits, stop_bits;
+ int speed, parity, data_bits, stop_bits, frame_size;
QEMUSerialSetParams ssp;
+ if (s->divider == 0)
+ return;
+
+ frame_size = 1;
if (s->lcr & 0x08) {
if (s->lcr & 0x10)
parity = 'E';
@@ -156,19 +237,21 @@ static void serial_update_parameters(SerialState *s)
parity = 'O';
} else {
parity = 'N';
+ frame_size = 0;
}
if (s->lcr & 0x04)
stop_bits = 2;
else
stop_bits = 1;
+
data_bits = (s->lcr & 0x03) + 5;
- if (s->divider == 0)
- return;
+ frame_size += data_bits + stop_bits;
speed = s->baudbase / s->divider;
ssp.speed = speed;
ssp.parity = parity;
ssp.data_bits = data_bits;
ssp.stop_bits = stop_bits;
+ s->char_transmit_time = ( ticks_per_sec / speed ) * frame_size;
qemu_chr_ioctl(s->chr, CHR_IOCTL_SERIAL_SET_PARAMS, &ssp);
#if 0
printf("speed=%d parity=%c data=%d stop=%d\n",
@@ -176,6 +259,88 @@ static void serial_update_parameters(SerialState *s)
#endif
}
+static void serial_update_msl( SerialState *s )
More bad whitespace.
+static void serial_xmit(void *opaque) {
+ SerialState *s = opaque;
+ uint64_t new_xmit_ts = qemu_get_clock(vm_clock);
+
+ if ( s->tsr_retry <= 0 ) {
+ if (s->fcr & UART_FCR_FE) {
+ s->tsr = fifo_get(s,XMIT_FIFO);
+ if ( !s->xmit_fifo.count )
+ s->lsr |= UART_LSR_THRE;
+ } else {
+ s->tsr = s->thr;
+ s->lsr |= UART_LSR_THRE;
+ }
+ }
+
+ if ( (s->mcr & UART_MCR_LOOP
+ /* in loopback mode, say that we just received a char */
+ ? (serial_receive1(s, &s->tsr, 1), 1)
+ : qemu_chr_write(s->chr, &s->tsr, 1))
This is just nutty :-) Please rewrite this if() statement to be a
little less obscure.
@@ -524,18 +832,11 @@ SerialState *serial_mm_init (target_phys_addr_t base, int
it_shift,
s = qemu_mallocz(sizeof(SerialState));
if (!s)
return NULL;
- s->irq = irq;
+
s->base = base;
s->it_shift = it_shift;
- s->baudbase= baudbase;
-
- s->tx_timer = qemu_new_timer(vm_clock, serial_tx_done, s);
- if (!s->tx_timer)
- return NULL;
-
- qemu_register_reset(serial_reset, s);
- serial_reset(s);
+ serial_init_core(s, irq, baudbase, chr);
register_savevm("serial", base, 2, serial_save, serial_load, s);
Isn't it necessary to bump the savevm() version number since you've
changed the format.
Regards,
Anthony Liguori
[Qemu-devel] [PATCH] upgrading emulated UART to 16550A, Stefano Stabellini, 2008/08/08
[Qemu-devel] [PATCH] upgrading emulated UART to 16550A, Stefano Stabellini, 2008/08/08