Re: [Qemu-devel] [PATCHv4 08/13] sparc32_dma: make esp device child of espdma device

[Philippe Mathieu-Daudé]

On 10/25/2017 12:59 PM, Mark Cave-Ayland wrote:
> This makes it possible to reference the esp device from the espdma device as
> required, and by wiring up the device ourselves in sun4m.c we can drop use
> of the esp_init() function.
> 
> Signed-off-by: Mark Cave-Ayland <address@hidden>
> Reviewed-by: Artyom Tarasenko <address@hidden>
> ---
>  hw/dma/sparc32_dma.c           |   26 ++++++++++++++++++++++++++
>  hw/sparc/sun4m.c               |   19 ++++++++-----------
>  include/hw/sparc/sparc32_dma.h |    3 +++
>  3 files changed, 37 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
> index c56a2ba..6009b94 100644
> --- a/hw/dma/sparc32_dma.c
> +++ b/hw/dma/sparc32_dma.c
> @@ -298,11 +298,37 @@ static void sparc32_espdma_device_init(Object *obj)
>      s->is_ledma = 0;
>  }
>  
> +static void sparc32_espdma_device_realize(DeviceState *dev, Error **errp)
> +{
> +    DeviceState *d;
> +    SysBusESPState *sysbus;
> +    ESPState *esp;
> +
> +    d = qdev_create(NULL, TYPE_ESP);
> +    object_property_add_child(OBJECT(dev), "esp", OBJECT(d), errp);

TYPE_ESP ?

> +    sysbus = ESP_STATE(d);
> +    esp = &sysbus->esp;
> +    esp->dma_memory_read = espdma_memory_read;
> +    esp->dma_memory_write = espdma_memory_write;
> +    esp->dma_opaque = SPARC32_DMA_DEVICE(dev);
> +    sysbus->it_shift = 2;
> +    esp->dma_enabled = 1;
> +    qdev_init_nofail(d);
> +}
> +
> +static void sparc32_espdma_device_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = sparc32_espdma_device_realize;
> +}
> +
>  static const TypeInfo sparc32_espdma_device_info = {
>      .name          = TYPE_SPARC32_ESPDMA_DEVICE,
>      .parent        = TYPE_SPARC32_DMA_DEVICE,
>      .instance_size = sizeof(ESPDMADeviceState),
>      .instance_init = sparc32_espdma_device_init,
> +    .class_init    = sparc32_espdma_device_class_init,
>  };
>  
>  static void sparc32_ledma_device_init(Object *obj)
> diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c
> index 12d36b5..4626c85 100644
> --- a/hw/sparc/sun4m.c
> +++ b/hw/sparc/sun4m.c
> @@ -819,10 +819,9 @@ static void sun4m_hw_init(const struct sun4m_hwdef 
> *hwdef,
>      const char *cpu_model = machine->cpu_model;
>      unsigned int i;
>      void *iommu, *nvram;
> -    DeviceState *espdma, *ledma;
> +    DeviceState *espdma, *esp, *ledma;
>      SysBusDevice *sbd;
>      qemu_irq *cpu_irqs[MAX_CPUS], slavio_irq[32], slavio_cpu_irq[MAX_CPUS];
> -    qemu_irq esp_reset, dma_enable;
>      qemu_irq fdc_tc;
>      unsigned long kernel_size;
>      DriveInfo *fd[MAX_FD];
> @@ -884,6 +883,13 @@ static void sun4m_hw_init(const struct sun4m_hwdef 
> *hwdef,
>      sbd = SYS_BUS_DEVICE(espdma);
>      sysbus_connect_irq(sbd, 0, slavio_irq[18]);
>  
> +    esp = DEVICE(object_resolve_path_component(OBJECT(espdma), "esp"));
> +    sbd = SYS_BUS_DEVICE(esp);
> +    sysbus_mmio_map(sbd, 0, hwdef->esp_base);
> +    sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(espdma, 0));
> +    qdev_connect_gpio_out(espdma, 0, qdev_get_gpio_in(esp, 0));
> +    qdev_connect_gpio_out(espdma, 1, qdev_get_gpio_in(esp, 1));
> +
>      ledma = sparc32_dma_init(hwdef->dma_base + 16ULL, iommu, 1);
>      sbd = SYS_BUS_DEVICE(ledma);
>      sysbus_connect_irq(sbd, 0, slavio_irq[16]);
> @@ -970,15 +976,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef 
> *hwdef,
>      slavio_misc_init(hwdef->slavio_base, hwdef->aux1_base, hwdef->aux2_base,
>                       slavio_irq[30], fdc_tc);
>  
> -    esp_init(hwdef->esp_base, 2,
> -             espdma_memory_read, espdma_memory_write,
> -             espdma,
> -             qdev_get_gpio_in(espdma, 0),
> -             &esp_reset, &dma_enable);
> -
> -    qdev_connect_gpio_out(espdma, 0, esp_reset);
> -    qdev_connect_gpio_out(espdma, 1, dma_enable);
> -
>      if (hwdef->cs_base) {
>          sysbus_create_simple("SUNW,CS4231", hwdef->cs_base,
>                               slavio_irq[5]);
> diff --git a/include/hw/sparc/sparc32_dma.h b/include/hw/sparc/sparc32_dma.h
> index df7491d..365160f 100644
> --- a/include/hw/sparc/sparc32_dma.h
> +++ b/include/hw/sparc/sparc32_dma.h
> @@ -2,6 +2,7 @@
>  #define SPARC32_DMA_H
>  
>  #include "hw/sysbus.h"
> +#include "hw/scsi/esp.h"

I'd keep this in "hw/sparc/sun4m_dma.h"

>  
>  #define DMA_REGS 4
>  
> @@ -28,6 +29,8 @@ struct DMADeviceState {
>  
>  typedef struct ESPDMADeviceState {
>      DMADeviceState parent_obj;
> +
> +    SysBusESPState *esp;
>  } ESPDMADeviceState;
>  
>  #define TYPE_SPARC32_LEDMA_DEVICE "sparc32-ledma"
>