Re: [Qemu-devel] [PATCH v2 07/17] dma/rc4030: convert to QOM

[Aurelien Jarno]

On 2015-05-27 14:19, Hervé Poussineau wrote:
> Signed-off-by: Hervé Poussineau <address@hidden>
> ---
>  hw/dma/rc4030.c        | 115 
> ++++++++++++++++++++++++++++++++++++++-----------
>  hw/mips/mips_jazz.c    |  37 ++++++++++------
>  include/hw/mips/mips.h |   4 +-
>  3 files changed, 113 insertions(+), 43 deletions(-)
> 
> diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
> index 55844ed..3efa6de 100644
> --- a/hw/dma/rc4030.c
> +++ b/hw/dma/rc4030.c
> @@ -1,7 +1,7 @@
>  /*
>   * QEMU JAZZ RC4030 chipset
>   *
> - * Copyright (c) 2007-2009 Herve Poussineau
> + * Copyright (c) 2007-2013 Hervé Poussineau
>   *
>   * Permission is hereby granted, free of charge, to any person obtaining a 
> copy
>   * of this software and associated documentation files (the "Software"), to 
> deal
> @@ -24,6 +24,7 @@
>  
>  #include "hw/hw.h"
>  #include "hw/mips/mips.h"
> +#include "hw/sysbus.h"
>  #include "qemu/timer.h"
>  #include "exec/address-spaces.h"
>  #include "trace.h"
> @@ -49,8 +50,14 @@ typedef struct dma_pagetable_entry {
>  #define DMA_FLAG_MEM_INTR   0x0200
>  #define DMA_FLAG_ADDR_INTR  0x0400
>  
> +#define TYPE_RC4030 "rc4030"
> +#define RC4030(obj) \
> +    OBJECT_CHECK(rc4030State, (obj), TYPE_RC4030)
> +
>  typedef struct rc4030State
>  {
> +    SysBusDevice parent;
> +
>      uint32_t config; /* 0x0000: RC4030 config register */
>      uint32_t revision; /* 0x0008: RC4030 Revision register */
>      uint32_t invalid_address_register; /* 0x0010: Invalid Address register */
> @@ -317,7 +324,7 @@ static void rc4030_dma_tt_update(rc4030State *s, uint32_t 
> new_tl_base,
>          } else {
>              dma_tt_size = memory_region_size(&s->dma_tt);
>          }
> -        memory_region_init_alias(&s->dma_tt_alias, NULL,
> +        memory_region_init_alias(&s->dma_tt_alias, OBJECT(s),
>                                   "dma-table-alias",
>                                   &s->dma_tt, 0, dma_tt_size);
>          dma_tl_contents = memory_region_get_ram_ptr(&s->dma_tt);
> @@ -332,7 +339,7 @@ static void rc4030_dma_tt_update(rc4030State *s, uint32_t 
> new_tl_base,
>                                      &s->dma_tt_alias);
>          memory_region_transaction_commit();
>      } else {
> -        memory_region_init(&s->dma_tt_alias, NULL,
> +        memory_region_init(&s->dma_tt_alias, OBJECT(s),
>                             "dma-table-alias", 0);
>      }
>  }
> @@ -577,9 +584,9 @@ static const MemoryRegionOps jazzio_ops = {
>      .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>  
> -static void rc4030_reset(void *opaque)
> +static void rc4030_reset(DeviceState *dev)
>  {
> -    rc4030State *s = opaque;
> +    rc4030State *s = RC4030(dev);
>      int i;
>  
>      s->config = 0x410; /* some boards seem to accept 0x104 too */
> @@ -733,46 +740,102 @@ static rc4030_dma *rc4030_allocate_dmas(void *opaque, 
> int n)
>      return s;
>  }
>  
> -MemoryRegion *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
> -                          qemu_irq **irqs, rc4030_dma **dmas,
> -                          MemoryRegion *sysmem)
> +static void rc4030_initfn(Object *obj)
>  {
> -    rc4030State *s;
> -    int i;
> -
> -    s = g_malloc0(sizeof(rc4030State));
> +    DeviceState *dev = DEVICE(obj);
> +    rc4030State *s = RC4030(obj);
> +    SysBusDevice *sysbus = SYS_BUS_DEVICE(obj);
>  
> -    *irqs = qemu_allocate_irqs(rc4030_irq_jazz_request, s, 16);
> -    *dmas = rc4030_allocate_dmas(s, 4);
> +    qdev_init_gpio_in(dev, rc4030_irq_jazz_request, 16);
>  
> -    s->periodic_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, 
> rc4030_periodic_timer, s);
> -    s->timer_irq = timer;
> -    s->jazz_bus_irq = jazz_bus;
> +    sysbus_init_irq(sysbus, &s->timer_irq);
> +    sysbus_init_irq(sysbus, &s->jazz_bus_irq);
>  
> -    qemu_register_reset(rc4030_reset, s);
>      register_savevm(NULL, "rc4030", 0, 2, rc4030_save, rc4030_load, s);
> -    rc4030_reset(s);
> +
> +    sysbus_init_mmio(sysbus, &s->iomem_chipset);
> +    sysbus_init_mmio(sysbus, &s->iomem_jazzio);
> +}
> +
> +static void rc4030_realize(DeviceState *dev, Error **errp)
> +{
> +    rc4030State *s = RC4030(dev);
> +    Object *o = OBJECT(dev);
> +    int i;
> +
> +    s->periodic_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> +                                     rc4030_periodic_timer, s);
>  
>      memory_region_init_io(&s->iomem_chipset, NULL, &rc4030_ops, s,
>                            "rc4030.chipset", 0x300);
> -    memory_region_add_subregion(sysmem, 0x80000000, &s->iomem_chipset);
>      memory_region_init_io(&s->iomem_jazzio, NULL, &jazzio_ops, s,
>                            "rc4030.jazzio", 0x00001000);
> -    memory_region_add_subregion(sysmem, 0xf0000000, &s->iomem_jazzio);
>  
> -    memory_region_init_rom_device(&s->dma_tt, NULL,
> +    memory_region_init_rom_device(&s->dma_tt, o,
>                                    &rc4030_dma_tt_ops, s, "dma-table",
>                                    MAX_TL_ENTRIES * 
> sizeof(dma_pagetable_entry),
>                                    NULL);
> -    memory_region_init(&s->dma_tt_alias, NULL, "dma-table-alias", 0);
> -    memory_region_init(&s->dma_mr, NULL, "dma", INT32_MAX);
> +    memory_region_init(&s->dma_tt_alias, o, "dma-table-alias", 0);
> +    memory_region_init(&s->dma_mr, o, "dma", INT32_MAX);
>      for (i = 0; i < MAX_TL_ENTRIES; ++i) {
> -        memory_region_init_alias(&s->dma_mrs[i], NULL, "dma-alias",
> +        memory_region_init_alias(&s->dma_mrs[i], o, "dma-alias",
>                                   get_system_memory(), 0, DMA_PAGESIZE);
>          memory_region_set_enabled(&s->dma_mrs[i], false);
>          memory_region_add_subregion(&s->dma_mr, i * DMA_PAGESIZE,
>                                      &s->dma_mrs[i]);
>      }
>      address_space_init(&s->dma_as, &s->dma_mr, "rc4030-dma");
> -    return &s->dma_mr;
> +}
> +
> +static void rc4030_unrealize(DeviceState *dev, Error **errp)
> +{
> +    rc4030State *s = RC4030(dev);
> +    int i;
> +
> +    timer_free(s->periodic_timer);
> +
> +    address_space_destroy(&s->dma_as);
> +    object_unparent(OBJECT(&s->dma_tt));
> +    object_unparent(OBJECT(&s->dma_tt_alias));
> +    object_unparent(OBJECT(&s->dma_mr));
> +    for (i = 0; i < MAX_TL_ENTRIES; ++i) {
> +        memory_region_del_subregion(&s->dma_mr, &s->dma_mrs[i]);
> +        object_unparent(OBJECT(&s->dma_mrs[i]));
> +    }
> +}
> +
> +static void rc4030_class_init(ObjectClass *klass, void *class_data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = rc4030_realize;
> +    dc->unrealize = rc4030_unrealize;
> +    dc->reset = rc4030_reset;
> +}
> +
> +static const TypeInfo rc4030_info = {
> +    .name = TYPE_RC4030,
> +    .parent = TYPE_SYS_BUS_DEVICE,
> +    .instance_size = sizeof(rc4030State),
> +    .instance_init = rc4030_initfn,
> +    .class_init = rc4030_class_init,
> +};
> +
> +static void rc4030_register_types(void)
> +{
> +    type_register_static(&rc4030_info);
> +}
> +
> +type_init(rc4030_register_types)
> +
> +DeviceState *rc4030_init(rc4030_dma **dmas, MemoryRegion **dma_mr)
> +{
> +    DeviceState *dev;
> +
> +    dev = qdev_create(NULL, TYPE_RC4030);
> +    qdev_init_nofail(dev);
> +
> +    *dmas = rc4030_allocate_dmas(dev, 4);
> +    *dma_mr = &RC4030(dev)->dma_mr;
> +    return dev;
>  }
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index 05cad6b..29a13c0 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -135,7 +135,7 @@ static void mips_jazz_init(MachineState *machine,
>      MIPSCPU *cpu;
>      CPUClass *cc;
>      CPUMIPSState *env;
> -    qemu_irq *rc4030, *i8259;
> +    qemu_irq *i8259;
>      rc4030_dma *dmas;
>      MemoryRegion *rc4030_dma_mr;
>      MemoryRegion *isa_mem = g_new(MemoryRegion, 1);
> @@ -144,7 +144,7 @@ static void mips_jazz_init(MachineState *machine,
>      MemoryRegion *i8042 = g_new(MemoryRegion, 1);
>      MemoryRegion *dma_dummy = g_new(MemoryRegion, 1);
>      NICInfo *nd;
> -    DeviceState *dev;
> +    DeviceState *dev, *rc4030;
>      SysBusDevice *sysbus;
>      ISABus *isa_bus;
>      ISADevice *pit;
> @@ -213,8 +213,14 @@ static void mips_jazz_init(MachineState *machine,
>      cpu_mips_clock_init(env);
>  
>      /* Chipset */
> -    rc4030_dma_mr = rc4030_init(env->irq[6], env->irq[3], &rc4030, &dmas,
> -                                address_space);
> +    rc4030 = rc4030_init(&dmas, &rc4030_dma_mr);
> +    sysbus = SYS_BUS_DEVICE(rc4030);
> +    sysbus_connect_irq(sysbus, 0, env->irq[6]);
> +    sysbus_connect_irq(sysbus, 1, env->irq[3]);
> +    memory_region_add_subregion(address_space, 0x80000000,
> +                                sysbus_mmio_get_region(sysbus, 0));
> +    memory_region_add_subregion(address_space, 0xf0000000,
> +                                sysbus_mmio_get_region(sysbus, 1));
>      memory_region_init_io(dma_dummy, NULL, &dma_dummy_ops, NULL, 
> "dummy_dma", 0x1000);
>      memory_region_add_subregion(address_space, 0x8000d000, dma_dummy);
>  
> @@ -241,7 +247,7 @@ static void mips_jazz_init(MachineState *machine,
>          sysbus = SYS_BUS_DEVICE(dev);
>          sysbus_mmio_map(sysbus, 0, 0x60080000);
>          sysbus_mmio_map(sysbus, 1, 0x40000000);
> -        sysbus_connect_irq(sysbus, 0, rc4030[3]);
> +        sysbus_connect_irq(sysbus, 0, qdev_get_gpio_in(rc4030, 3));
>          {
>              /* Simple ROM, so user doesn't have to provide one */
>              MemoryRegion *rom_mr = g_new(MemoryRegion, 1);
> @@ -267,8 +273,8 @@ static void mips_jazz_init(MachineState *machine,
>          if (!nd->model)
>              nd->model = g_strdup("dp83932");
>          if (strcmp(nd->model, "dp83932") == 0) {
> -            dp83932_init(nd, 0x80001000, 2, get_system_memory(), rc4030[4],
> -                         rc4030_dma_mr);
> +            dp83932_init(nd, 0x80001000, 2, get_system_memory(),
> +                         qdev_get_gpio_in(rc4030, 4), rc4030_dma_mr);
>              break;
>          } else if (is_help_option(nd->model)) {
>              fprintf(stderr, "qemu: Supported NICs: dp83932\n");
> @@ -282,7 +288,7 @@ static void mips_jazz_init(MachineState *machine,
>      /* SCSI adapter */
>      esp_init(0x80002000, 0,
>               rc4030_dma_read, rc4030_dma_write, dmas[0],
> -             rc4030[5], &esp_reset, &dma_enable);
> +             qdev_get_gpio_in(rc4030, 5), &esp_reset, &dma_enable);
>  
>      /* Floppy */
>      if (drive_get_max_bus(IF_FLOPPY) >= MAX_FD) {
> @@ -292,7 +298,7 @@ static void mips_jazz_init(MachineState *machine,
>      for (n = 0; n < MAX_FD; n++) {
>          fds[n] = drive_get(IF_FLOPPY, 0, n);
>      }
> -    fdctrl_init_sysbus(rc4030[1], 0, 0x80003000, fds);
> +    fdctrl_init_sysbus(qdev_get_gpio_in(rc4030, 1), 0, 0x80003000, fds);
>  
>      /* Real time clock */
>      rtc_init(isa_bus, 1980, NULL);
> @@ -300,23 +306,26 @@ static void mips_jazz_init(MachineState *machine,
>      memory_region_add_subregion(address_space, 0x80004000, rtc);
>  
>      /* Keyboard (i8042) */
> -    i8042_mm_init(rc4030[6], rc4030[7], i8042, 0x1000, 0x1);
> +    i8042_mm_init(qdev_get_gpio_in(rc4030, 6), qdev_get_gpio_in(rc4030, 7),
> +                  i8042, 0x1000, 0x1);
>      memory_region_add_subregion(address_space, 0x80005000, i8042);
>  
>      /* Serial ports */
>      if (serial_hds[0]) {
> -        serial_mm_init(address_space, 0x80006000, 0, rc4030[8], 8000000/16,
> +        serial_mm_init(address_space, 0x80006000, 0,
> +                       qdev_get_gpio_in(rc4030, 8), 8000000/16,
>                         serial_hds[0], DEVICE_NATIVE_ENDIAN);
>      }
>      if (serial_hds[1]) {
> -        serial_mm_init(address_space, 0x80007000, 0, rc4030[9], 8000000/16,
> +        serial_mm_init(address_space, 0x80007000, 0,
> +                       qdev_get_gpio_in(rc4030, 9), 8000000/16,
>                         serial_hds[1], DEVICE_NATIVE_ENDIAN);
>      }
>  
>      /* Parallel port */
>      if (parallel_hds[0])
> -        parallel_mm_init(address_space, 0x80008000, 0, rc4030[0],
> -                         parallel_hds[0]);
> +        parallel_mm_init(address_space, 0x80008000, 0,
> +                         qdev_get_gpio_in(rc4030, 0), parallel_hds[0]);
>  
>      /* FIXME: missing Jazz sound at 0x8000c000, rc4030[2] */
>  
> diff --git a/include/hw/mips/mips.h b/include/hw/mips/mips.h
> index 47eb31f..31b4729 100644
> --- a/include/hw/mips/mips.h
> +++ b/include/hw/mips/mips.h
> @@ -18,9 +18,7 @@ typedef struct rc4030DMAState *rc4030_dma;
>  void rc4030_dma_read(void *dma, uint8_t *buf, int len);
>  void rc4030_dma_write(void *dma, uint8_t *buf, int len);
>  
> -MemoryRegion *rc4030_init(qemu_irq timer, qemu_irq jazz_bus,
> -                          qemu_irq **irqs, rc4030_dma **dmas,
> -                          MemoryRegion *sysmem);
> +DeviceState *rc4030_init(rc4030_dma **dmas, MemoryRegion **dma_mr);
>  
>  /* dp8393x.c */
>  void dp83932_init(NICInfo *nd, hwaddr base, int it_shift,

I am not very comfortable with the QOM API, but it looks fine to me.

Reviewed-by: Aurelien Jarno <address@hidden>


-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
address@hidden                 http://www.aurel32.net