From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1JlTYG-000795-NE for qemu-devel@nongnu.org; Mon, 14 Apr 2008 14:37:12 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1JlTYE-00078t-43 for qemu-devel@nongnu.org; Mon, 14 Apr 2008 14:37:11 -0400 Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1JlTYD-00078q-Tv for qemu-devel@nongnu.org; Mon, 14 Apr 2008 14:37:09 -0400 Received: from ug-out-1314.google.com ([66.249.92.175]) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1JlTYC-0000JA-Ve for qemu-devel@nongnu.org; Mon, 14 Apr 2008 14:37:09 -0400 Received: by ug-out-1314.google.com with SMTP id m2so536935uge.4 for ; Mon, 14 Apr 2008 11:37:06 -0700 (PDT) Message-ID: Date: Mon, 14 Apr 2008 20:37:05 +0200 From: "andrzej zaborowski" Subject: Re: [Qemu-devel] [PATCH 2/2] 8250: Customized base baudrate In-Reply-To: <4801C476.3040509@web.de> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <4801C476.3040509@web.de> Reply-To: qemu-devel@nongnu.org List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org On 13/04/2008, Jan Kiszka wrote: > Previous patch revealed another issue of the 8250 emulation: The base > baudrate is assumed to be 115200. This is surely not correct and could > already cause wrong configurations if the virtual UART is forwarded to > real one. With the previous patch applied, a wrong baudbase is even more > problematic as it could lead to wrong throttling. > > This patch extends serial[_mm]_init by a basebaud parameter and attempts > to fix all call sites - without claiming to be correct in all cases (I > tried to grab the information from current Linux). Please verify. > > Signed-off-by: Jan Kiszka > --- > hw/mips_jazz.c | 4 ++-- > hw/mips_malta.c | 7 ++++--- > hw/mips_mipssim.c | 2 +- > hw/mips_r4k.c | 3 ++- > hw/omap1.c | 3 ++- > hw/pc.c | 3 ++- > hw/pc.h | 7 ++++--- > hw/ppc405_uc.c | 2 +- > hw/ppc_chrp.c | 2 +- > hw/ppc_oldworld.c | 2 +- > hw/ppc_prep.c | 2 +- > hw/pxa2xx.c | 6 ++++-- > hw/serial.c | 14 +++++++++----- > 13 files changed, 34 insertions(+), 23 deletions(-) > > Index: b/hw/serial.c > =================================================================== > --- a/hw/serial.c > +++ b/hw/serial.c > @@ -99,6 +99,7 @@ struct SerialState { > int last_break_enable; > target_phys_addr_t base; > int it_shift; > + int baudbase; > QEMUTimer *tx_timer; > int tx_burst; > char tx_buf; > @@ -129,7 +130,7 @@ static void serial_tx_done(void *opaque) > if (s->tx_burst < 0) { > /* We assume 10 bits/char, OK for this purpose. */ > s->tx_burst = THROTTLE_TX_INTERVAL * 1000 / > - (1000000 * 10 / (115200 / s->divider)); > + (1000000 * 10 / (s->baudbase / s->divider)); > } > s->thr_ipending = 1; > s->lsr |= UART_LSR_THRE; > @@ -161,7 +162,7 @@ static void serial_update_parameters(Ser > data_bits = (s->lcr & 0x03) + 5; > if (s->divider == 0) > return; > - speed = 115200 / s->divider; > + speed = s->baudbase / s->divider; > ssp.speed = speed; > ssp.parity = parity; > ssp.data_bits = data_bits; > @@ -407,7 +408,8 @@ static void serial_reset(void *opaque) > } > > /* If fd is zero, it means that the serial device uses the console */ > -SerialState *serial_init(int base, qemu_irq irq, CharDriverState *chr) > +SerialState *serial_init(int base, qemu_irq irq, int baudbase, > + CharDriverState *chr) > { > SerialState *s; > > @@ -415,6 +417,7 @@ SerialState *serial_init(int base, qemu_ > if (!s) > return NULL; > s->irq = irq; > + s->baudbase = baudbase; > > s->tx_timer = qemu_new_timer(vm_clock, serial_tx_done, s); > if (!s->tx_timer) > @@ -506,8 +509,8 @@ static CPUWriteMemoryFunc *serial_mm_wri > }; > > SerialState *serial_mm_init (target_phys_addr_t base, int it_shift, > - qemu_irq irq, CharDriverState *chr, > - int ioregister) > + qemu_irq irq, int baudbase, > + CharDriverState *chr, int ioregister) > { > SerialState *s; > int s_io_memory; > @@ -518,6 +521,7 @@ SerialState *serial_mm_init (target_phys > 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) > Index: b/hw/omap1.c > =================================================================== > --- a/hw/omap1.c > +++ b/hw/omap1.c > @@ -1734,7 +1734,8 @@ struct omap_uart_s *omap_uart_init(targe > struct omap_uart_s *s = (struct omap_uart_s *) > qemu_mallocz(sizeof(struct omap_uart_s)); > if (chr) > - s->serial = serial_mm_init(base, 2, irq, chr, 1); > + s->serial = > + serial_mm_init(base, 2, irq, omap_clk_getrate(clk)/16, chr, 1); Yes, this should be correct. > return s; > } > > Index: b/hw/pc.c > =================================================================== > --- a/hw/pc.c > +++ b/hw/pc.c > @@ -909,7 +909,8 @@ static void pc_init1(int ram_size, int v > > for(i = 0; i < MAX_SERIAL_PORTS; i++) { > if (serial_hds[i]) { > - serial_init(serial_io[i], i8259[serial_irq[i]], serial_hds[i]); > + serial_init(serial_io[i], i8259[serial_irq[i]], 115200, > + serial_hds[i]); > } > } > > Index: b/hw/pc.h > =================================================================== > --- a/hw/pc.h > +++ b/hw/pc.h > @@ -4,10 +4,11 @@ > > /* serial.c */ > > -SerialState *serial_init(int base, qemu_irq irq, CharDriverState *chr); > +SerialState *serial_init(int base, qemu_irq irq, int baudbase, > + CharDriverState *chr); > SerialState *serial_mm_init (target_phys_addr_t base, int it_shift, > - qemu_irq irq, CharDriverState *chr, > - int ioregister); > + qemu_irq irq, int baudbase, > + CharDriverState *chr, int ioregister); > uint32_t serial_mm_readb (void *opaque, target_phys_addr_t addr); > void serial_mm_writeb (void *opaque, target_phys_addr_t addr, uint32_t value); > uint32_t serial_mm_readw (void *opaque, target_phys_addr_t addr); > Index: b/hw/pxa2xx.c > =================================================================== > --- a/hw/pxa2xx.c > +++ b/hw/pxa2xx.c > @@ -2077,7 +2077,8 @@ struct pxa2xx_state_s *pxa270_init(unsig > for (i = 0; pxa270_serial[i].io_base; i ++) > if (serial_hds[i]) > serial_mm_init(pxa270_serial[i].io_base, 2, > - s->pic[pxa270_serial[i].irqn], serial_hds[i], 1); > + s->pic[pxa270_serial[i].irqn], 14857000/16, > + serial_hds[i], 1); > else > break; > if (serial_hds[i]) > @@ -2202,7 +2203,8 @@ struct pxa2xx_state_s *pxa255_init(unsig > for (i = 0; pxa255_serial[i].io_base; i ++) > if (serial_hds[i]) > serial_mm_init(pxa255_serial[i].io_base, 2, > - s->pic[pxa255_serial[i].irqn], serial_hds[i], 1); > + s->pic[pxa255_serial[i].irqn], 14745600/16, > + serial_hds[i], 1); This also (although I, too, only know from looking at Linux code). > else > break; > if (serial_hds[i]) > Index: b/hw/mips_jazz.c > =================================================================== > --- a/hw/mips_jazz.c > +++ b/hw/mips_jazz.c > @@ -234,9 +234,9 @@ void mips_jazz_init (int ram_size, int v > > /* Serial ports */ > if (serial_hds[0]) > - serial_mm_init(0x80006000, 0, rc4030[8], serial_hds[0], 1); > + serial_mm_init(0x80006000, 0, rc4030[8], 8000000/16, serial_hds[0], 1); > if (serial_hds[1]) > - serial_mm_init(0x80007000, 0, rc4030[9], serial_hds[1], 1); > + serial_mm_init(0x80007000, 0, rc4030[9], 8000000/16, serial_hds[1], 1); > > /* Parallel port */ > if (parallel_hds[0]) > Index: b/hw/mips_malta.c > =================================================================== > --- a/hw/mips_malta.c > +++ b/hw/mips_malta.c > @@ -449,7 +449,8 @@ static MaltaFPGAState *malta_fpga_init(t > > uart_chr = qemu_chr_open("vc"); > qemu_chr_printf(uart_chr, "CBUS UART\r\n"); > - s->uart = serial_mm_init(base + 0x900, 3, env->irq[2], uart_chr, 1); > + s->uart = > + serial_mm_init(base + 0x900, 3, env->irq[2], 230400, uart_chr, 1); > > malta_fpga_reset(s); > qemu_register_reset(malta_fpga_reset, s); > @@ -918,9 +919,9 @@ void mips_malta_init (int ram_size, int > i8042_init(i8259[1], i8259[12], 0x60); > rtc_state = rtc_init(0x70, i8259[8]); > if (serial_hds[0]) > - serial_init(0x3f8, i8259[4], serial_hds[0]); > + serial_init(0x3f8, i8259[4], 115200, serial_hds[0]); > if (serial_hds[1]) > - serial_init(0x2f8, i8259[3], serial_hds[1]); > + serial_init(0x2f8, i8259[3], 115200, serial_hds[1]); > if (parallel_hds[0]) > parallel_init(0x378, i8259[7], parallel_hds[0]); > for(i = 0; i < MAX_FD; i++) { > Index: b/hw/mips_mipssim.c > =================================================================== > --- a/hw/mips_mipssim.c > +++ b/hw/mips_mipssim.c > @@ -174,7 +174,7 @@ mips_mipssim_init (int ram_size, int vga > /* A single 16450 sits at offset 0x3f8. It is attached to > MIPS CPU INT2, which is interrupt 4. */ > if (serial_hds[0]) > - serial_init(0x3f8, env->irq[4], serial_hds[0]); > + serial_init(0x3f8, env->irq[4], 115200, serial_hds[0]); > > if (nd_table[0].vlan) { > if (nd_table[0].model == NULL > Index: b/hw/mips_r4k.c > =================================================================== > --- a/hw/mips_r4k.c > +++ b/hw/mips_r4k.c > @@ -229,7 +229,8 @@ void mips_r4k_init (int ram_size, int vg > > for(i = 0; i < MAX_SERIAL_PORTS; i++) { > if (serial_hds[i]) { > - serial_init(serial_io[i], i8259[serial_irq[i]], serial_hds[i]); > + serial_init(serial_io[i], i8259[serial_irq[i]], 115200, > + serial_hds[i]); > } > } > > Index: b/hw/ppc405_uc.c > =================================================================== > --- a/hw/ppc405_uc.c > +++ b/hw/ppc405_uc.c > @@ -1223,7 +1223,7 @@ void ppc405_serial_init (CPUState *env, > #ifdef DEBUG_SERIAL > printf("%s: offset " PADDRX "\n", __func__, offset); > #endif > - serial = serial_mm_init(offset, 0, irq, chr, 0); > + serial = serial_mm_init(offset, 0, irq, 399193, chr, 0); > ppc4xx_mmio_register(env, mmio, offset, 0x008, > serial_mm_read, serial_mm_write, serial); > } > Index: b/hw/ppc_chrp.c > =================================================================== > --- a/hw/ppc_chrp.c > +++ b/hw/ppc_chrp.c > @@ -264,7 +264,7 @@ static void ppc_core99_init (int ram_siz > dummy_irq = i8259_init(NULL); > > /* XXX: use Mac Serial port */ > - serial_init(0x3f8, dummy_irq[4], serial_hds[0]); > + serial_init(0x3f8, dummy_irq[4], 115200, serial_hds[0]); > for(i = 0; i < nb_nics; i++) { > if (!nd_table[i].model) > nd_table[i].model = "ne2k_pci"; > Index: b/hw/ppc_oldworld.c > =================================================================== > --- a/hw/ppc_oldworld.c > +++ b/hw/ppc_oldworld.c > @@ -287,7 +287,7 @@ static void ppc_heathrow_init (int ram_s > dummy_irq = i8259_init(NULL); > > /* XXX: use Mac Serial port */ > - serial_init(0x3f8, dummy_irq[4], serial_hds[0]); > + serial_init(0x3f8, dummy_irq[4], 115200, serial_hds[0]); > > for(i = 0; i < nb_nics; i++) { > if (!nd_table[i].model) > Index: b/hw/ppc_prep.c > =================================================================== > --- a/hw/ppc_prep.c > +++ b/hw/ppc_prep.c > @@ -667,7 +667,7 @@ static void ppc_prep_init (int ram_size, > // pit = pit_init(0x40, i8259[0]); > rtc_init(0x70, i8259[8]); > > - serial_init(0x3f8, i8259[4], serial_hds[0]); > + serial_init(0x3f8, i8259[4], 115200, serial_hds[0]); > nb_nics1 = nb_nics; > if (nb_nics1 > NE2000_NB_MAX) > nb_nics1 = NE2000_NB_MAX; > > > -- Please do not print this email unless absolutely necessary. Spread environmental awareness.