| Message ID | CAErn8fSCPZ1YKnf0OWq98S3-GFK9-3=uWgwfMO5Hut7PiDpcrw@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Corrected memory regions | expand |
Hi Seth, On Wed, Nov 14, 2018 at 8:25 AM Seth K <skintigh@gmail.com> wrote: > > I corrected these 2 memory regions based on specifications from the chip > manufacturer. The existing ranges seem to overlap and and cause odd > behavior and/or crashes when trying to set up multiple UARTs, > I also played with changing MAX_SERIAL_PORTS to 8 to match the hardware, > but I did not include that in this patch as I never fully tested its > effects. > This is my first patch, I hope I did it correctly, > Seth Kintigh Welcome to QEMU, and thanks for your patch :) I checked the DocID15818 (Rev 15) datasheet for this chip and your patch is technically correct. I suggest the patch title as "hw/arm/stm32f205: Fix the UART and Timer region size" (note: use present rather than past). However for the legal stuff, the QEMU project states all patches "must include a Signed-off-by: line". Please have a look at the following page which explains everything you need to know in order to send useful contributions: https://wiki.qemu.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_Signed-off-by:_line > --- Note than the commit message stay in the repository history forever. You can add information not related to the patch below the previous "---" line, it will be cut out when your patch is applied to the repository. Here would go your "This is my first patch, I hope I did it correctly" and signature. Regards, Phil. > hw/char/stm32f2xx_usart.c | 2 +- > hw/timer/stm32f2xx_timer.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/char/stm32f2xx_usart.c b/hw/char/stm32f2xx_usart.c > index 032b5fda13..f3363a2952 100644 > --- a/hw/char/stm32f2xx_usart.c > +++ b/hw/char/stm32f2xx_usart.c > @@ -202,7 +202,7 @@ static void stm32f2xx_usart_init(Object *obj) > sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq); > > memory_region_init_io(&s->mmio, obj, &stm32f2xx_usart_ops, s, > - TYPE_STM32F2XX_USART, 0x2000); > + TYPE_STM32F2XX_USART, 0x400); > sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio); > } > > diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c > index 58fc7b1188..ae744d1642 100644 > --- a/hw/timer/stm32f2xx_timer.c > +++ b/hw/timer/stm32f2xx_timer.c > @@ -308,7 +308,7 @@ static void stm32f2xx_timer_init(Object *obj) > sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq); > > memory_region_init_io(&s->iomem, obj, &stm32f2xx_timer_ops, s, > - "stm32f2xx_timer", 0x4000); > + "stm32f2xx_timer", 0x400); > sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem); > > s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, stm32f2xx_timer_interrupt, > s); > -- > 2.11.0
On 4 November 2018 at 07:42, Seth K <skintigh@gmail.com> wrote: > I corrected these 2 memory regions based on specifications from the chip > manufacturer. The existing ranges seem to overlap and and cause odd > behavior and/or crashes when trying to set up multiple UARTs, > I also played with changing MAX_SERIAL_PORTS to 8 to match the hardware, > but I did not include that in this patch as I never fully tested its > effects. Hi; thanks for the patch. As Philippe says, it looks good, but the one thing we definitely need is a Signed-off-by: line from you. A minor note -- there is no "MAX_SERIAL_PORTS" definition in QEMU any more: we removed that artificial limitation earlier this year. Maybe you're basing your patch on an older version of QEMU? It's best to use git master for development. Our SoC model defines 6 uarts (STM_NUM_USARTS) in hw/arm/stm32f205_soc.c, which should all now be connectable on the command line, though I haven't tested that or checked whether the hardware has 6 or some other number... thanks -- PMM
diff --git a/hw/char/stm32f2xx_usart.c b/hw/char/stm32f2xx_usart.c index 032b5fda13..f3363a2952 100644 --- a/hw/char/stm32f2xx_usart.c +++ b/hw/char/stm32f2xx_usart.c @@ -202,7 +202,7 @@ static void stm32f2xx_usart_init(Object *obj) sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq); memory_region_init_io(&s->mmio, obj, &stm32f2xx_usart_ops, s, - TYPE_STM32F2XX_USART, 0x2000); + TYPE_STM32F2XX_USART, 0x400); sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio); } diff --git a/hw/timer/stm32f2xx_timer.c b/hw/timer/stm32f2xx_timer.c index 58fc7b1188..ae744d1642 100644 --- a/hw/timer/stm32f2xx_timer.c +++ b/hw/timer/stm32f2xx_timer.c @@ -308,7 +308,7 @@ static void stm32f2xx_timer_init(Object *obj) sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq); memory_region_init_io(&s->iomem, obj, &stm32f2xx_timer_ops, s, - "stm32f2xx_timer", 0x4000); + "stm32f2xx_timer", 0x400); sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem); s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, stm32f2xx_timer_interrupt,