Re: [Qemu-devel] [PATCH v10 4/5] stm32f205: Add the stm32f205 SoC

[Peter Crosthwaite]

On Thu, Jan 29, 2015 at 4:31 AM, Alistair Francis <address@hidden> wrote:
> This patch adds the stm32f205 SoC. This will be used by the
> Netduino 2 to create a machine.
>
> Signed-off-by: Alistair Francis <address@hidden>
> ---
> V6:
>  - Correct the number of USART/UART devices
>  - Use macros to define how many devices are inited
>  - Update the memory regions name from netduino.* to
>    STM32F205.*
>
>  default-configs/arm-softmmu.mak |   1 +
>  hw/arm/Makefile.objs            |   1 +
>  hw/arm/stm32f205_soc.c          | 157 
> ++++++++++++++++++++++++++++++++++++++++
>  include/hw/arm/stm32f205_soc.h  |  69 ++++++++++++++++++
>  4 files changed, 228 insertions(+)
>  create mode 100644 hw/arm/stm32f205_soc.c
>  create mode 100644 include/hw/arm/stm32f205_soc.h
>
> diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
> index a5aab7f..9ac755e 100644
> --- a/default-configs/arm-softmmu.mak
> +++ b/default-configs/arm-softmmu.mak
> @@ -81,6 +81,7 @@ CONFIG_ZYNQ=y
>  CONFIG_STM32F2XX_TIMER=y
>  CONFIG_STM32F2XX_USART=y
>  CONFIG_STM32F2XX_SYSCFG=y
> +CONFIG_STM32F205_SOC=y
>
>  CONFIG_VERSATILE_PCI=y
>  CONFIG_VERSATILE_I2C=y
> diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
> index 6088e53..9769317 100644
> --- a/hw/arm/Makefile.objs
> +++ b/hw/arm/Makefile.objs
> @@ -8,3 +8,4 @@ obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o 
> pxa2xx_pic.o
>  obj-$(CONFIG_DIGIC) += digic.o
>  obj-y += omap1.o omap2.o strongarm.o
>  obj-$(CONFIG_ALLWINNER_A10) += allwinner-a10.o cubieboard.o
> +obj-$(CONFIG_STM32F205_SOC) += stm32f205_soc.o
> diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
> new file mode 100644
> index 0000000..186e15d
> --- /dev/null
> +++ b/hw/arm/stm32f205_soc.c
> @@ -0,0 +1,157 @@
> +/*
> + * STM32F205 SoC
> + *
> + * Copyright (c) 2014 Alistair Francis <address@hidden>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#include "hw/arm/stm32f205_soc.h"
> +
> +/* At the moment only Timer 2 to 5 are modelled */
> +static const uint32_t timer_addr[STM_NUM_TIMERS] = { 0x40000000, 0x40000400,
> +    0x40000800, 0x40000C00 };
> +static const uint32_t usart_addr[STM_NUM_USARTS] = { 0x40011000, 0x40004400,
> +    0x40004800, 0x40004C00, 0x40005000, 0x40011400 };
> +
> +static const int timer_irq[STM_NUM_TIMERS] = {28, 29, 30, 50};
> +static const int usart_irq[STM_NUM_USARTS] = {37, 38, 39, 52, 53, 71};
> +
> +static void stm32f205_soc_initfn(Object *obj)
> +{
> +    STM32F205State *s = STM32F205_SOC(obj);
> +    int i;
> +
> +    object_initialize(&s->syscfg, sizeof(s->syscfg), TYPE_STM32F2XX_SYSCFG);
> +    qdev_set_parent_bus(DEVICE(&s->syscfg), sysbus_get_default());
> +
> +    for (i = 0; i < STM_NUM_USARTS; i++) {
> +        object_initialize(&s->usart[i], sizeof(s->usart[i]),
> +                          TYPE_STM32F2XX_USART);
> +        qdev_set_parent_bus(DEVICE(&s->usart[i]), sysbus_get_default());
> +    }
> +
> +    for (i = 0; i < STM_NUM_TIMERS; i++) {
> +        object_initialize(&s->timer[i], sizeof(s->timer[i]),
> +                          TYPE_STM32F2XX_TIMER);
> +        qdev_set_parent_bus(DEVICE(&s->timer[i]), sysbus_get_default());
> +    }
> +}
> +
> +static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
> +{
> +    STM32F205State *s = STM32F205_SOC(dev_soc);
> +    DeviceState *syscfgdev, *usartdev, *timerdev;
> +    SysBusDevice *syscfgbusdev, *usartbusdev, *timerbusdev;
> +    qemu_irq *pic;
> +    Error *err = NULL;
> +    int i;
> +
> +    MemoryRegion *system_memory = get_system_memory();
> +    MemoryRegion *sram = g_new(MemoryRegion, 1);
> +    MemoryRegion *flash = g_new(MemoryRegion, 1);
> +    MemoryRegion *flash_alias = g_new(MemoryRegion, 1);
> +
> +    memory_region_init_ram(flash, NULL, "STM32F205.flash", FLASH_SIZE,
> +                           &error_abort);
> +    memory_region_init_alias(flash_alias, NULL, "STM32F205.flash.alias",
> +                             flash, 0, FLASH_SIZE);
> +
> +    vmstate_register_ram_global(flash);
> +
> +    memory_region_set_readonly(flash, true);
> +    memory_region_set_readonly(flash_alias, true);
> +
> +    memory_region_add_subregion(system_memory, FLASH_BASE_ADDRESS, flash);
> +    memory_region_add_subregion(system_memory, 0, flash_alias);
> +
> +    memory_region_init_ram(sram, NULL, "STM32F205.sram", SRAM_SIZE,
> +                           &error_abort);
> +    vmstate_register_ram_global(sram);
> +    memory_region_add_subregion(system_memory, SRAM_BASE_ADDRESS, sram);
> +
> +    pic = armv7m_init(get_system_memory(), FLASH_SIZE, 96,
> +                      s->kernel_filename, s->cpu_model);
> +
> +    /* System configuration controller */
> +    syscfgdev = DEVICE(&s->syscfg);
> +    object_property_set_bool(OBJECT(&s->syscfg), true, "realized", &err);
> +    if (err != NULL) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    syscfgbusdev = SYS_BUS_DEVICE(syscfgdev);
> +    sysbus_mmio_map(syscfgbusdev, 0, 0x40013800);
> +    sysbus_connect_irq(syscfgbusdev, 0, pic[71]);
> +
> +    /* Attach UART (uses USART registers) and USART controllers */
> +    for (i = 0; i < STM_NUM_USARTS; i++) {
> +        usartdev = DEVICE(&(s->usart[i]));
> +        object_property_set_bool(OBJECT(&s->usart[i]), true, "realized", 
> &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +        usartbusdev = SYS_BUS_DEVICE(usartdev);
> +        sysbus_mmio_map(usartbusdev, 0, usart_addr[i]);
> +        sysbus_connect_irq(usartbusdev, 0, pic[usart_irq[i]]);
> +    }
> +
> +    /* Timer 2 to 5 */
> +    for (i = 0; i < STM_NUM_TIMERS; i++) {
> +        timerdev = DEVICE(&(s->timer[i]));
> +        qdev_prop_set_uint64(timerdev, "clock-frequency", 1000000000);
> +        object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", 
> &err);
> +        if (err != NULL) {
> +            error_propagate(errp, err);
> +            return;
> +        }
> +        timerbusdev = SYS_BUS_DEVICE(timerdev);
> +        sysbus_mmio_map(timerbusdev, 0, timer_addr[i]);
> +        sysbus_connect_irq(timerbusdev, 0, pic[timer_irq[i]]);
> +    }
> +}
> +
> +static Property stm32f205_soc_properties[] = {
> +    DEFINE_PROP_STRING("kernel-filename", STM32F205State, kernel_filename),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void stm32f205_soc_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = stm32f205_soc_realize;
> +    dc->props = stm32f205_soc_properties;
> +}
> +
> +static const TypeInfo stm32f205_soc_info = {
> +    .name          = TYPE_STM32F205_SOC,
> +    .parent        = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(STM32F205State),
> +    .instance_init = stm32f205_soc_initfn,
> +    .class_init    = stm32f205_soc_class_init,
> +};
> +
> +static void stm32f205_soc_types(void)
> +{
> +    type_register_static(&stm32f205_soc_info);
> +}
> +
> +type_init(stm32f205_soc_types)
> diff --git a/include/hw/arm/stm32f205_soc.h b/include/hw/arm/stm32f205_soc.h
> new file mode 100644
> index 0000000..4b71c40
> --- /dev/null
> +++ b/include/hw/arm/stm32f205_soc.h
> @@ -0,0 +1,69 @@
> +/*
> + * STM32F205 SoC
> + *
> + * Copyright (c) 2014 Alistair Francis <address@hidden>
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
> + * of this software and associated documentation files (the "Software"), to 
> deal
> + * in the Software without restriction, including without limitation the 
> rights
> + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
> + * copies of the Software, and to permit persons to whom the Software is
> + * furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
> FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
> + * THE SOFTWARE.
> + */
> +
> +#ifndef HW_ARM_STM32F205SOC_H
> +#define HW_ARM_STM32F205SOC_H
> +
> +#include "hw/sysbus.h"
> +#include "hw/arm/arm.h"
> +#include "hw/ssi.h"

Is this needed?

> +#include "hw/devices.h"
> +#include "qemu/timer.h"
> +#include "net/net.h"

Is this needed?

> +#include "elf.h"
> +#include "hw/loader.h"

And these.

I think the headers need to be audited for unneeded ones.

> +#include "hw/boards.h"
> +#include "exec/address-spaces.h"
> +#include "qemu/error-report.h"

Also as a general rule, you should limit the header included headers
to the ones that are needed by the header itself. Headers like this
that are only needed for the operation of the C file, can be included
in C files.

> +#include "sysemu/qtest.h"
> +#include "hw/misc/stm32f2xx_syscfg.h"
> +#include "hw/timer/stm32f2xx_timer.h"
> +#include "hw/char/stm32f2xx_usart.h"
> +
> +#define TYPE_STM32F205_SOC "stm32f205_soc"
> +#define STM32F205_SOC(obj) \
> +    OBJECT_CHECK(STM32F205State, (obj), TYPE_STM32F205_SOC)
> +
> +#define STM_NUM_USARTS 6
> +#define STM_NUM_TIMERS 4
> +
> +#define FLASH_BASE_ADDRESS 0x08000000
> +#define FLASH_SIZE (1024 * 1024)
> +#define SRAM_BASE_ADDRESS 0x20000000
> +#define SRAM_SIZE (128 * 1024)
> +
> +typedef struct STM32F205State {
> +    /*< private >*/
> +    SysBusDevice parent_obj;
> +    /*< public >*/
> +
> +    char *kernel_filename;
> +    char *cpu_model;

What sets this? Can you delete it and just pass NULL to ARMv7 init?

Regards,
Peter

> +
> +    STM32F2XXSyscfgState syscfg;
> +    STM32F2XXUsartState usart[STM_NUM_USARTS];
> +    STM32F2XXTimerState timer[STM_NUM_TIMERS];
> +} STM32F205State;
> +
> +#endif
> --
> 2.1.0
>
>