[Qemu-devel] [PATCH 0/5] isa: more qdev conversions.

[Qemu-devel] [PATCH 0/5] isa: more qdev conversions.

[Gerd Hoffmann]

  Hi,

This patch series converts more isa devices to qdev.

cheers,
  Gerd

[Qemu-devel] [PATCH 1/5] floppy: add drive properties.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/fdc.c        |   50 +++++++++++++++++++++++++++++++++++++-------------
 hw/fdc.h        |    7 ++++---
 hw/mips_jazz.c  |    5 ++---
 hw/mips_malta.c |    5 ++---
 hw/pc.c         |    6 ++----
 hw/ppc_prep.c   |    6 ++----
 hw/sun4m.c      |   16 ++++------------
 hw/sun4u.c      |    6 ++----
 8 files changed, 55 insertions(+), 46 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index 389d9e6..ea3b8ac 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -81,6 +81,7 @@ typedef enum fdisk_flags_t {
 } fdisk_flags_t;
 
 typedef struct fdrive_t {
+    DriveInfo *dinfo;
     BlockDriverState *bs;
     /* Drive status */
     fdrive_type_t drive;
@@ -97,10 +98,10 @@ typedef struct fdrive_t {
     uint8_t ro;               /* Is read-only           */
 } fdrive_t;
 
-static void fd_init (fdrive_t *drv, BlockDriverState *bs)
+static void fd_init (fdrive_t *drv)
 {
     /* Drive */
-    drv->bs = bs;
+    drv->bs = drv->dinfo ? drv->dinfo->bdrv : NULL;
     drv->drive = FDRIVE_DRV_NONE;
     drv->perpendicular = 0;
     /* Disk */
@@ -1829,43 +1830,50 @@ static void fdctrl_result_timer(void *opaque)
 }
 
 /* Init functions */
-static void fdctrl_connect_drives(fdctrl_t *fdctrl, BlockDriverState **fds)
+static void fdctrl_connect_drives(fdctrl_t *fdctrl)
 {
     unsigned int i;
 
     for (i = 0; i < MAX_FD; i++) {
-        fd_init(&fdctrl->drives[i], fds[i]);
+        fd_init(&fdctrl->drives[i]);
         fd_revalidate(&fdctrl->drives[i]);
     }
 }
 
-fdctrl_t *fdctrl_init_isa(BlockDriverState **fds)
+fdctrl_t *fdctrl_init_isa(DriveInfo **fds)
 {
     fdctrl_t *fdctrl;
     ISADevice *dev;
     int dma_chann = 2;
 
-    dev = isa_create_simple("isa-fdc");
+    dev = isa_create("isa-fdc");
+    qdev_prop_set_drive(&dev->qdev, "driveA", fds[0]);
+    qdev_prop_set_drive(&dev->qdev, "driveB", fds[1]);
+    if (qdev_init(&dev->qdev) != 0)
+        return NULL;
     fdctrl = &(DO_UPCAST(fdctrl_isabus_t, busdev, dev)->state);
 
     fdctrl->dma_chann = dma_chann;
     DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
 
-    fdctrl_connect_drives(fdctrl, fds);
+    fdctrl_connect_drives(fdctrl);
 
     return fdctrl;
 }
 
 fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
                              target_phys_addr_t mmio_base,
-                             BlockDriverState **fds)
+                             DriveInfo **fds)
 {
     fdctrl_t *fdctrl;
     DeviceState *dev;
     fdctrl_sysbus_t *sys;
 
     dev = qdev_create(NULL, "sysbus-fdc");
-    qdev_init(dev);
+    qdev_prop_set_drive(dev, "driveA", fds[0]);
+    qdev_prop_set_drive(dev, "driveB", fds[1]);
+    if (qdev_init(dev) != 0)
+        return NULL;
     sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
     fdctrl = &sys->state;
     sysbus_connect_irq(&sys->busdev, 0, irq);
@@ -1873,20 +1881,22 @@ fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
 
     fdctrl->dma_chann = dma_chann;
     DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
-    fdctrl_connect_drives(fdctrl, fds);
+    fdctrl_connect_drives(fdctrl);
 
     return fdctrl;
 }
 
 fdctrl_t *sun4m_fdctrl_init (qemu_irq irq, target_phys_addr_t io_base,
-                             BlockDriverState **fds, qemu_irq *fdc_tc)
+                             DriveInfo **fds, qemu_irq *fdc_tc)
 {
     DeviceState *dev;
     fdctrl_sysbus_t *sys;
     fdctrl_t *fdctrl;
 
     dev = qdev_create(NULL, "SUNW,fdtwo");
-    qdev_init(dev);
+    qdev_prop_set_drive(dev, "drive", fds[0]);
+    if (qdev_init(dev) != 0)
+        return NULL;
     sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
     fdctrl = &sys->state;
     sysbus_connect_irq(&sys->busdev, 0, irq);
@@ -1895,7 +1905,7 @@ fdctrl_t *sun4m_fdctrl_init (qemu_irq irq, target_phys_addr_t io_base,
 
     fdctrl->dma_chann = -1;
 
-    fdctrl_connect_drives(fdctrl, fds);
+    fdctrl_connect_drives(fdctrl);
 
     return fdctrl;
 }
@@ -1985,18 +1995,32 @@ static ISADeviceInfo isa_fdc_info = {
     .init = isabus_fdc_init1,
     .qdev.name  = "isa-fdc",
     .qdev.size  = sizeof(fdctrl_isabus_t),
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_DRIVE("driveA", fdctrl_isabus_t, state.drives[0].dinfo),
+        DEFINE_PROP_DRIVE("driveB", fdctrl_isabus_t, state.drives[1].dinfo),
+        DEFINE_PROP_END_OF_LIST(),
+    },
 };
 
 static SysBusDeviceInfo sysbus_fdc_info = {
     .init = sysbus_fdc_init1,
     .qdev.name  = "sysbus-fdc",
     .qdev.size  = sizeof(fdctrl_sysbus_t),
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_DRIVE("driveA", fdctrl_sysbus_t, state.drives[0].dinfo),
+        DEFINE_PROP_DRIVE("driveB", fdctrl_sysbus_t, state.drives[1].dinfo),
+        DEFINE_PROP_END_OF_LIST(),
+    },
 };
 
 static SysBusDeviceInfo sun4m_fdc_info = {
     .init = sun4m_fdc_init1,
     .qdev.name  = "SUNW,fdtwo",
     .qdev.size  = sizeof(fdctrl_sysbus_t),
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_DRIVE("drive", fdctrl_sysbus_t, state.drives[0].dinfo),
+        DEFINE_PROP_END_OF_LIST(),
+    },
 };
 
 static void fdc_register_devices(void)
diff --git a/hw/fdc.h b/hw/fdc.h
index 1b81ec1..c64e8b4 100644
--- a/hw/fdc.h
+++ b/hw/fdc.h
@@ -1,12 +1,13 @@
 /* fdc.c */
+#include "sysemu.h"
 #define MAX_FD 2
 
 typedef struct fdctrl_t fdctrl_t;
 
-fdctrl_t *fdctrl_init_isa(BlockDriverState **fds);
+fdctrl_t *fdctrl_init_isa(DriveInfo **fds);
 fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
                              target_phys_addr_t mmio_base,
-                             BlockDriverState **fds);
+                             DriveInfo **fds);
 fdctrl_t *sun4m_fdctrl_init (qemu_irq irq, target_phys_addr_t io_base,
-                             BlockDriverState **fds, qemu_irq *fdc_tc);
+                             DriveInfo **fds, qemu_irq *fdc_tc);
 int fdctrl_get_drive_type(fdctrl_t *fdctrl, int drive_num);
diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
index d62a584..ace551b 100644
--- a/hw/mips_jazz.c
+++ b/hw/mips_jazz.c
@@ -127,7 +127,7 @@ void mips_jazz_init (ram_addr_t ram_size,
     int s_rtc, s_dma_dummy;
     NICInfo *nd;
     PITState *pit;
-    BlockDriverState *fds[MAX_FD];
+    DriveInfo *fds[MAX_FD];
     qemu_irq esp_reset;
     ram_addr_t ram_offset;
     ram_addr_t bios_offset;
@@ -235,8 +235,7 @@ void mips_jazz_init (ram_addr_t ram_size,
         exit(1);
     }
     for (n = 0; n < MAX_FD; n++) {
-        DriveInfo *dinfo = drive_get(IF_FLOPPY, 0, n);
-        fds[n] = dinfo ? dinfo->bdrv : NULL;
+        fds[n] = drive_get(IF_FLOPPY, 0, n);
     }
     fdctrl_init_sysbus(rc4030[1], 0, 0x80003000, fds);
 
diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index 0a6eaa4..260f266 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -777,7 +777,7 @@ void mips_malta_init (ram_addr_t ram_size,
     int i;
     DriveInfo *dinfo;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-    BlockDriverState *fd[MAX_FD];
+    DriveInfo *fd[MAX_FD];
     int fl_idx = 0;
     int fl_sectors = 0;
 
@@ -938,8 +938,7 @@ void mips_malta_init (ram_addr_t ram_size,
     if (parallel_hds[0])
         parallel_init(0x378, isa_reserve_irq(7), parallel_hds[0]);
     for(i = 0; i < MAX_FD; i++) {
-        dinfo = drive_get(IF_FLOPPY, 0, i);
-        fd[i] = dinfo ? dinfo->bdrv : NULL;
+        fd[i] = drive_get(IF_FLOPPY, 0, i);
     }
     floppy_controller = fdctrl_init_isa(fd);
 
diff --git a/hw/pc.c b/hw/pc.c
index 58de372..8865ec0 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1130,9 +1130,8 @@ static void pc_init1(ram_addr_t ram_size,
     qemu_irq *isa_irq;
     qemu_irq *i8259;
     IsaIrqState *isa_irq_state;
-    DriveInfo *dinfo;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-    BlockDriverState *fd[MAX_FD];
+    DriveInfo *fd[MAX_FD];
     int using_vga = cirrus_vga_enabled || std_vga_enabled || vmsvga_enabled;
     void *fw_cfg;
 
@@ -1378,8 +1377,7 @@ static void pc_init1(ram_addr_t ram_size,
 #endif
 
     for(i = 0; i < MAX_FD; i++) {
-        dinfo = drive_get(IF_FLOPPY, 0, i);
-        fd[i] = dinfo ? dinfo->bdrv : NULL;
+        fd[i] = drive_get(IF_FLOPPY, 0, i);
     }
     floppy_controller = fdctrl_init_isa(fd);
 
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 5392072..32f987c 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -562,9 +562,8 @@ static void ppc_prep_init (ram_addr_t ram_size,
     PCIBus *pci_bus;
     qemu_irq *i8259;
     int ppc_boot_device;
-    DriveInfo *dinfo;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-    BlockDriverState *fd[MAX_FD];
+    DriveInfo *fd[MAX_FD];
 
     sysctrl = qemu_mallocz(sizeof(sysctrl_t));
 
@@ -719,8 +718,7 @@ static void ppc_prep_init (ram_addr_t ram_size,
     //    SB16_init();
 
     for(i = 0; i < MAX_FD; i++) {
-        dinfo = drive_get(IF_FLOPPY, 0, i);
-        fd[i] = dinfo ? dinfo->bdrv : NULL;
+        fd[i] = drive_get(IF_FLOPPY, 0, i);
     }
     fdctrl_init_isa(fd);
 
diff --git a/hw/sun4m.c b/hw/sun4m.c
index d970723..6d5d4e8 100644
--- a/hw/sun4m.c
+++ b/hw/sun4m.c
@@ -747,9 +747,8 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
     qemu_irq fdc_tc;
     qemu_irq *cpu_halt;
     unsigned long kernel_size;
-    BlockDriverState *fd[MAX_FD];
+    DriveInfo *fd[MAX_FD];
     void *fw_cfg;
-    DriveInfo *dinfo;
 
     /* init CPUs */
     if (!cpu_model)
@@ -823,10 +822,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
     if (hwdef->fd_base) {
         /* there is zero or one floppy drive */
         memset(fd, 0, sizeof(fd));
-        dinfo = drive_get(IF_FLOPPY, 0, 0);
-        if (dinfo)
-            fd[0] = dinfo->bdrv;
-
+        fd[0] = drive_get(IF_FLOPPY, 0, 0);
         sun4m_fdctrl_init(slavio_irq[22], hwdef->fd_base, fd,
                           &fdc_tc);
     }
@@ -1551,11 +1547,10 @@ static void sun4c_hw_init(const struct sun4c_hwdef *hwdef, ram_addr_t RAM_size,
     qemu_irq esp_reset;
     qemu_irq fdc_tc;
     unsigned long kernel_size;
-    BlockDriverState *fd[MAX_FD];
+    DriveInfo *fd[MAX_FD];
     void *fw_cfg;
     DeviceState *dev;
     unsigned int i;
-    DriveInfo *dinfo;
 
     /* init CPU */
     if (!cpu_model)
@@ -1607,10 +1602,7 @@ static void sun4c_hw_init(const struct sun4c_hwdef *hwdef, ram_addr_t RAM_size,
     if (hwdef->fd_base != (target_phys_addr_t)-1) {
         /* there is zero or one floppy drive */
         memset(fd, 0, sizeof(fd));
-        dinfo = drive_get(IF_FLOPPY, 0, 0);
-        if (dinfo)
-            fd[0] = dinfo->bdrv;
-
+        fd[0] = drive_get(IF_FLOPPY, 0, 0);
         sun4m_fdctrl_init(slavio_irq[1], hwdef->fd_base, fd,
                           &fdc_tc);
     }
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 427ee76..81ddf48 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -561,9 +561,8 @@ static void sun4uv_init(ram_addr_t RAM_size,
     PCIBus *pci_bus, *pci_bus2, *pci_bus3;
     qemu_irq *irq;
     DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS];
-    BlockDriverState *fd[MAX_FD];
+    DriveInfo *fd[MAX_FD];
     void *fw_cfg;
-    DriveInfo *dinfo;
 
     /* init CPUs */
     env = cpu_devinit(cpu_model, hwdef);
@@ -619,8 +618,7 @@ static void sun4uv_init(ram_addr_t RAM_size,
 
     isa_create_simple("i8042");
     for(i = 0; i < MAX_FD; i++) {
-        dinfo = drive_get(IF_FLOPPY, 0, i);
-        fd[i] = dinfo ? dinfo->bdrv : NULL;
+        fd[i] = drive_get(IF_FLOPPY, 0, i);
     }
     fdctrl_init_isa(fd);
     nvram = m48t59_init_isa(0x0074, NVRAM_SIZE, 59);
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/5] floppy: move dma setup + drive connect to fdctrl_init_common()

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/fdc.c |   33 ++++++++++++---------------------
 1 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index ea3b8ac..537db66 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1842,23 +1842,14 @@ static void fdctrl_connect_drives(fdctrl_t *fdctrl)
 
 fdctrl_t *fdctrl_init_isa(DriveInfo **fds)
 {
-    fdctrl_t *fdctrl;
     ISADevice *dev;
-    int dma_chann = 2;
 
     dev = isa_create("isa-fdc");
     qdev_prop_set_drive(&dev->qdev, "driveA", fds[0]);
     qdev_prop_set_drive(&dev->qdev, "driveB", fds[1]);
     if (qdev_init(&dev->qdev) != 0)
         return NULL;
-    fdctrl = &(DO_UPCAST(fdctrl_isabus_t, busdev, dev)->state);
-
-    fdctrl->dma_chann = dma_chann;
-    DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
-
-    fdctrl_connect_drives(fdctrl);
-
-    return fdctrl;
+    return &(DO_UPCAST(fdctrl_isabus_t, busdev, dev)->state);
 }
 
 fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
@@ -1870,19 +1861,16 @@ fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
     fdctrl_sysbus_t *sys;
 
     dev = qdev_create(NULL, "sysbus-fdc");
+    sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
+    fdctrl = &sys->state;
+    fdctrl->dma_chann = dma_chann; /* FIXME */
     qdev_prop_set_drive(dev, "driveA", fds[0]);
     qdev_prop_set_drive(dev, "driveB", fds[1]);
     if (qdev_init(dev) != 0)
         return NULL;
-    sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
-    fdctrl = &sys->state;
     sysbus_connect_irq(&sys->busdev, 0, irq);
     sysbus_mmio_map(&sys->busdev, 0, mmio_base);
 
-    fdctrl->dma_chann = dma_chann;
-    DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
-    fdctrl_connect_drives(fdctrl);
-
     return fdctrl;
 }
 
@@ -1901,11 +1889,7 @@ fdctrl_t *sun4m_fdctrl_init (qemu_irq irq, target_phys_addr_t io_base,
     fdctrl = &sys->state;
     sysbus_connect_irq(&sys->busdev, 0, irq);
     sysbus_mmio_map(&sys->busdev, 0, io_base);
-    *fdc_tc = qdev_get_gpio_in(dev, 0);
-
-    fdctrl->dma_chann = -1;
-
-    fdctrl_connect_drives(fdctrl);
+    *fdc_tc = qdev_get_gpio_in(dev, 0); /* FIXME */
 
     return fdctrl;
 }
@@ -1937,6 +1921,10 @@ static int fdctrl_init_common(fdctrl_t *fdctrl)
     fdctrl->config = FD_CONFIG_EIS | FD_CONFIG_EFIFO; /* Implicit seek, polling & FIFO enabled */
     fdctrl->num_floppies = MAX_FD;
 
+    if (fdctrl->dma_chann != -1)
+        DMA_register_channel(fdctrl->dma_chann, &fdctrl_transfer_handler, fdctrl);
+    fdctrl_connect_drives(fdctrl);
+
     fdctrl_external_reset(fdctrl);
     vmstate_register(-1, &vmstate_fdc, fdctrl);
     qemu_register_reset(fdctrl_external_reset, fdctrl);
@@ -1949,6 +1937,7 @@ static int isabus_fdc_init1(ISADevice *dev)
     fdctrl_t *fdctrl = &isa->state;
     int iobase = 0x3f0;
     int isairq = 6;
+    int dma_chann = 2;
 
     register_ioport_read(iobase + 0x01, 5, 1,
                          &fdctrl_read_port, fdctrl);
@@ -1959,6 +1948,7 @@ static int isabus_fdc_init1(ISADevice *dev)
     register_ioport_write(iobase + 0x07, 1, 1,
                           &fdctrl_write_port, fdctrl);
     isa_init_irq(&isa->busdev, &fdctrl->irq, isairq);
+    fdctrl->dma_chann = dma_chann;
 
     return fdctrl_init_common(fdctrl);
 }
@@ -1972,6 +1962,7 @@ static int sysbus_fdc_init1(SysBusDevice *dev)
     sysbus_init_mmio(dev, 0x08, io);
     sysbus_init_irq(dev, &fdctrl->irq);
     qdev_init_gpio_in(&dev->qdev, fdctrl_handle_tc, 1);
+    fdctrl->dma_chann = -1;
 
     return fdctrl_init_common(fdctrl);
 }
-- 
1.6.2.5

[Qemu-devel] [PATCH 3/5] qdev: don't crash on unset drive properties.

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/qdev-properties.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 2ecb58d..edacf35 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -180,7 +180,7 @@ static int parse_drive(DeviceState *dev, Property *prop, const char *str)
 static int print_drive(DeviceState *dev, Property *prop, char *dest, size_t len)
 {
     DriveInfo **ptr = qdev_get_prop_ptr(dev, prop);
-    return snprintf(dest, len, "%s", (*ptr)->id);
+    return snprintf(dest, len, "%s", (*ptr) ? (*ptr)->id : "<null>");
 }
 
 PropertyInfo qdev_prop_drive = {
-- 
1.6.2.5

[Qemu-devel] [PATCH 4/5] serial: convert isa to qdev

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/mips_malta.c |    4 +-
 hw/mips_r4k.c   |    6 +---
 hw/pc.c         |    6 +---
 hw/pc.h         |    1 +
 hw/ppc_prep.c   |    3 +-
 hw/serial.c     |   77 +++++++++++++++++++++++++++++++++++++++++++++++-------
 hw/sun4u.c      |    6 +---
 7 files changed, 75 insertions(+), 28 deletions(-)

diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index 260f266..ed11b45 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -933,8 +933,8 @@ void mips_malta_init (ram_addr_t ram_size,
     isa_dev = isa_create_simple("i8042");
  
     rtc_state = rtc_init(2000);
-    serial_init(0x3f8, isa_reserve_irq(4), 115200, serial_hds[0]);
-    serial_init(0x2f8, isa_reserve_irq(3), 115200, serial_hds[1]);
+    serial_isa_init(0, serial_hds[0]);
+    serial_isa_init(1, serial_hds[1]);
     if (parallel_hds[0])
         parallel_init(0x378, isa_reserve_irq(7), parallel_hds[0]);
     for(i = 0; i < MAX_FD; i++) {
diff --git a/hw/mips_r4k.c b/hw/mips_r4k.c
index fcc7fed..4c94568 100644
--- a/hw/mips_r4k.c
+++ b/hw/mips_r4k.c
@@ -29,9 +29,6 @@ static const int ide_iobase[2] = { 0x1f0, 0x170 };
 static const int ide_iobase2[2] = { 0x3f6, 0x376 };
 static const int ide_irq[2] = { 14, 15 };
 
-static int serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
-static int serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
-
 static PITState *pit; /* PIT i8254 */
 
 /* i8254 PIT is attached to the IRQ0 at PIC i8259 */
@@ -254,8 +251,7 @@ void mips_r4k_init (ram_addr_t ram_size,
 
     for(i = 0; i < MAX_SERIAL_PORTS; i++) {
         if (serial_hds[i]) {
-            serial_init(serial_io[i], i8259[serial_irq[i]], 115200,
-                        serial_hds[i]);
+            serial_isa_init(i, serial_hds[i]);
         }
     }
 
diff --git a/hw/pc.c b/hw/pc.c
index 8865ec0..052c0a2 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1019,9 +1019,6 @@ static const int ne2000_io[NE2000_NB_MAX] = { 0x300, 0x320, 0x340, 0x360,
                                               0x280, 0x380 };
 static const int ne2000_irq[NE2000_NB_MAX] = { 9, 10, 11, 3, 4, 5 };
 
-static const int serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
-static const int serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
-
 static const int parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
 static const int parallel_irq[MAX_PARALLEL_PORTS] = { 7, 7, 7 };
 
@@ -1329,8 +1326,7 @@ static void pc_init1(ram_addr_t ram_size,
 
     for(i = 0; i < MAX_SERIAL_PORTS; i++) {
         if (serial_hds[i]) {
-            serial_init(serial_io[i], isa_reserve_irq(serial_irq[i]), 115200,
-                        serial_hds[i]);
+            serial_isa_init(i, serial_hds[i]);
         }
     }
 
diff --git a/hw/pc.h b/hw/pc.h
index c9cdd4a..b547d17 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -12,6 +12,7 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase,
 SerialState *serial_mm_init (target_phys_addr_t base, int it_shift,
                              qemu_irq irq, int baudbase,
                              CharDriverState *chr, int ioregister);
+SerialState *serial_isa_init(int index, CharDriverState *chr);
 
 /* parallel.c */
 
diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
index 32f987c..3f57cbb 100644
--- a/hw/ppc_prep.c
+++ b/hw/ppc_prep.c
@@ -684,7 +684,8 @@ static void ppc_prep_init (ram_addr_t ram_size,
     //    pit = pit_init(0x40, i8259[0]);
     rtc_init(2000);
 
-    serial_init(0x3f8, i8259[4], 115200, serial_hds[0]);
+    if (serial_hds[0])
+        serial_isa_init(0, serial_hds[0]);
     nb_nics1 = nb_nics;
     if (nb_nics1 > NE2000_NB_MAX)
         nb_nics1 = NE2000_NB_MAX;
diff --git a/hw/serial.c b/hw/serial.c
index 1f4ce77..81021e9 100644
--- a/hw/serial.c
+++ b/hw/serial.c
@@ -146,6 +146,13 @@ struct SerialState {
     struct QEMUTimer *modem_status_poll;
 };
 
+typedef struct ISASerialState {
+    ISADevice dev;
+    uint32_t iobase;
+    uint32_t isairq;
+    SerialState state;
+} ISASerialState;
+
 static void serial_receive1(void *opaque, const uint8_t *buf, int size);
 
 static void fifo_clear(SerialState *s, int fifo)
@@ -709,18 +716,13 @@ static void serial_reset(void *opaque)
     qemu_irq_lower(s->irq);
 }
 
-static void serial_init_core(SerialState *s, qemu_irq irq, int baudbase,
-			     CharDriverState *chr)
+static void serial_init_core(SerialState *s)
 {
-    if (!chr) {
+    if (!s->chr) {
         fprintf(stderr, "Can't create serial device, empty char device\n");
 	exit(1);
     }
 
-    s->irq = irq;
-    s->baudbase = baudbase;
-    s->chr = chr;
-
     s->modem_status_poll = qemu_new_timer(vm_clock, (QEMUTimerCB *) serial_update_msl, s);
 
     s->fifo_timeout_timer = qemu_new_timer(vm_clock, (QEMUTimerCB *) fifo_timeout_int, s);
@@ -733,7 +735,37 @@ static void serial_init_core(SerialState *s, qemu_irq irq, int baudbase,
                           serial_event, s);
 }
 
-/* If fd is zero, it means that the serial device uses the console */
+static int serial_isa_initfn(ISADevice *dev)
+{
+    ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
+    SerialState *s = &isa->state;
+
+    s->baudbase = 115200;
+    isa_init_irq(dev, &s->irq, isa->isairq);
+    serial_init_core(s);
+    vmstate_register(isa->iobase, &vmstate_serial, s);
+
+    register_ioport_write(isa->iobase, 8, 1, serial_ioport_write, s);
+    register_ioport_read(isa->iobase, 8, 1, serial_ioport_read, s);
+    return 0;
+}
+
+static const int isa_serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
+static const int isa_serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
+
+SerialState *serial_isa_init(int index, CharDriverState *chr)
+{
+    ISADevice *dev;
+
+    dev = isa_create("isa-serial");
+    qdev_prop_set_uint32(&dev->qdev, "iobase", isa_serial_io[index]);
+    qdev_prop_set_uint32(&dev->qdev, "irq", isa_serial_irq[index]);
+    qdev_prop_set_chr(&dev->qdev, "chardev", chr);
+    if (qdev_init(&dev->qdev) != 0)
+        return NULL;
+    return &DO_UPCAST(ISASerialState, dev, dev)->state;
+}
+
 SerialState *serial_init(int base, qemu_irq irq, int baudbase,
                          CharDriverState *chr)
 {
@@ -741,7 +773,10 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase,
 
     s = qemu_mallocz(sizeof(SerialState));
 
-    serial_init_core(s, irq, baudbase, chr);
+    s->irq = irq;
+    s->baudbase = baudbase;
+    s->chr = chr;
+    serial_init_core(s);
 
     vmstate_register(base, &vmstate_serial, s);
 
@@ -832,8 +867,11 @@ SerialState *serial_mm_init (target_phys_addr_t base, int it_shift,
     s = qemu_mallocz(sizeof(SerialState));
 
     s->it_shift = it_shift;
+    s->irq = irq;
+    s->baudbase = baudbase;
+    s->chr = chr;
 
-    serial_init_core(s, irq, baudbase, chr);
+    serial_init_core(s);
     vmstate_register(base, &vmstate_serial, s);
 
     if (ioregister) {
@@ -844,3 +882,22 @@ SerialState *serial_mm_init (target_phys_addr_t base, int it_shift,
     serial_update_msl(s);
     return s;
 }
+
+static ISADeviceInfo serial_isa_info = {
+    .qdev.name  = "isa-serial",
+    .qdev.size  = sizeof(ISASerialState),
+    .init       = serial_isa_initfn,
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_HEX32("iobase", ISASerialState, iobase,  0x3f8),
+        DEFINE_PROP_UINT32("irq",   ISASerialState, isairq,  4),
+        DEFINE_PROP_CHR("chardev",  ISASerialState, state.chr),
+        DEFINE_PROP_END_OF_LIST(),
+    },
+};
+
+static void serial_register_devices(void)
+{
+    isa_qdev_register(&serial_isa_info);
+}
+
+device_init(serial_register_devices)
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 81ddf48..2b55cf3 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -326,9 +326,6 @@ void cpu_tick_set_limit(void *opaque, uint64_t limit)
     ptimer_set_limit(opaque, -limit, 0);
 }
 
-static const int serial_io[MAX_SERIAL_PORTS] = { 0x3f8, 0x2f8, 0x3e8, 0x2e8 };
-static const int serial_irq[MAX_SERIAL_PORTS] = { 4, 3, 4, 3 };
-
 static const int parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
 static const int parallel_irq[MAX_PARALLEL_PORTS] = { 7, 7, 7 };
 
@@ -590,8 +587,7 @@ static void sun4uv_init(ram_addr_t RAM_size,
     }
     for(; i < MAX_SERIAL_PORTS; i++) {
         if (serial_hds[i]) {
-            serial_init(serial_io[i], NULL/*serial_irq[i]*/, 115200,
-                        serial_hds[i]);
+            serial_isa_init(i, serial_hds[i]);
         }
     }
 
-- 
1.6.2.5

[Qemu-devel] [PATCH 5/5] parallel: convert isa to qdev

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/mips_malta.c |    2 +-
 hw/parallel.c   |   80 ++++++++++++++++++++++++++++++++++++++++++------------
 hw/pc.c         |    3 +-
 hw/pc.h         |    2 +-
 hw/sun4u.c      |    3 +-
 5 files changed, 66 insertions(+), 24 deletions(-)

diff --git a/hw/mips_malta.c b/hw/mips_malta.c
index ed11b45..e348c1e 100644
--- a/hw/mips_malta.c
+++ b/hw/mips_malta.c
@@ -936,7 +936,7 @@ void mips_malta_init (ram_addr_t ram_size,
     serial_isa_init(0, serial_hds[0]);
     serial_isa_init(1, serial_hds[1]);
     if (parallel_hds[0])
-        parallel_init(0x378, isa_reserve_irq(7), parallel_hds[0]);
+        parallel_init(0, parallel_hds[0]);
     for(i = 0; i < MAX_FD; i++) {
         fd[i] = drive_get(IF_FLOPPY, 0, i);
     }
diff --git a/hw/parallel.c b/hw/parallel.c
index faaaa0d..30c53a2 100644
--- a/hw/parallel.c
+++ b/hw/parallel.c
@@ -78,6 +78,13 @@ struct ParallelState {
     int it_shift;
 };
 
+typedef struct ISAParallelState {
+    ISADevice dev;
+    uint32_t iobase;
+    uint32_t isairq;
+    ParallelState state;
+} ISAParallelState;
+
 static void parallel_update_irq(ParallelState *s)
 {
     if (s->irq_pending)
@@ -438,38 +445,56 @@ static void parallel_reset(void *opaque)
     s->last_read_offset = ~0U;
 }
 
-/* If fd is zero, it means that the parallel device uses the console */
-ParallelState *parallel_init(int base, qemu_irq irq, CharDriverState *chr)
+static int parallel_isa_initfn(ISADevice *dev)
 {
-    ParallelState *s;
+    ISAParallelState *isa = DO_UPCAST(ISAParallelState, dev, dev);
+    ParallelState *s = &isa->state;
     uint8_t dummy;
 
-    s = qemu_mallocz(sizeof(ParallelState));
-    s->irq = irq;
-    s->chr = chr;
+    if (!s->chr) {
+        fprintf(stderr, "Can't create parallel device, empty char device\n");
+        exit(1);
+    }
+
+    isa_init_irq(dev, &s->irq, isa->isairq);
     parallel_reset(s);
     qemu_register_reset(parallel_reset, s);
 
-    if (qemu_chr_ioctl(chr, CHR_IOCTL_PP_READ_STATUS, &dummy) == 0) {
+    if (qemu_chr_ioctl(s->chr, CHR_IOCTL_PP_READ_STATUS, &dummy) == 0) {
         s->hw_driver = 1;
         s->status = dummy;
     }
 
     if (s->hw_driver) {
-        register_ioport_write(base, 8, 1, parallel_ioport_write_hw, s);
-        register_ioport_read(base, 8, 1, parallel_ioport_read_hw, s);
-        register_ioport_write(base+4, 1, 2, parallel_ioport_eppdata_write_hw2, s);
-        register_ioport_read(base+4, 1, 2, parallel_ioport_eppdata_read_hw2, s);
-        register_ioport_write(base+4, 1, 4, parallel_ioport_eppdata_write_hw4, s);
-        register_ioport_read(base+4, 1, 4, parallel_ioport_eppdata_read_hw4, s);
-        register_ioport_write(base+0x400, 8, 1, parallel_ioport_ecp_write, s);
-        register_ioport_read(base+0x400, 8, 1, parallel_ioport_ecp_read, s);
+        register_ioport_write(isa->iobase, 8, 1, parallel_ioport_write_hw, s);
+        register_ioport_read(isa->iobase, 8, 1, parallel_ioport_read_hw, s);
+        register_ioport_write(isa->iobase+4, 1, 2, parallel_ioport_eppdata_write_hw2, s);
+        register_ioport_read(isa->iobase+4, 1, 2, parallel_ioport_eppdata_read_hw2, s);
+        register_ioport_write(isa->iobase+4, 1, 4, parallel_ioport_eppdata_write_hw4, s);
+        register_ioport_read(isa->iobase+4, 1, 4, parallel_ioport_eppdata_read_hw4, s);
+        register_ioport_write(isa->iobase+0x400, 8, 1, parallel_ioport_ecp_write, s);
+        register_ioport_read(isa->iobase+0x400, 8, 1, parallel_ioport_ecp_read, s);
     }
     else {
-        register_ioport_write(base, 8, 1, parallel_ioport_write_sw, s);
-        register_ioport_read(base, 8, 1, parallel_ioport_read_sw, s);
+        register_ioport_write(isa->iobase, 8, 1, parallel_ioport_write_sw, s);
+        register_ioport_read(isa->iobase, 8, 1, parallel_ioport_read_sw, s);
     }
-    return s;
+    return 0;
+}
+
+static const int isa_parallel_io[MAX_PARALLEL_PORTS] = { 0x378, 0x278, 0x3bc };
+
+ParallelState *parallel_init(int index, CharDriverState *chr)
+{
+    ISADevice *dev;
+
+    dev = isa_create("isa-parallel");
+    qdev_prop_set_uint32(&dev->qdev, "iobase", isa_parallel_io[index]);
+    qdev_prop_set_uint32(&dev->qdev, "irq", 7);
+    qdev_prop_set_chr(&dev->qdev, "chardev", chr);
+    if (qdev_init(&dev->qdev) != 0)
+        return NULL;
+    return &DO_UPCAST(ISAParallelState, dev, dev)->state;
 }
 
 /* Memory mapped interface */
@@ -547,3 +572,22 @@ ParallelState *parallel_mm_init(target_phys_addr_t base, int it_shift, qemu_irq
     cpu_register_physical_memory(base, 8 << it_shift, io_sw);
     return s;
 }
+
+static ISADeviceInfo parallel_isa_info = {
+    .qdev.name  = "isa-parallel",
+    .qdev.size  = sizeof(ISAParallelState),
+    .init       = parallel_isa_initfn,
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_HEX32("iobase", ISAParallelState, iobase,  0x378),
+        DEFINE_PROP_UINT32("irq",   ISAParallelState, isairq,  7),
+        DEFINE_PROP_CHR("chardev",  ISAParallelState, state.chr),
+        DEFINE_PROP_END_OF_LIST(),
+    },
+};
+
+static void parallel_register_devices(void)
+{
+    isa_qdev_register(&parallel_isa_info);
+}
+
+device_init(parallel_register_devices)
diff --git a/hw/pc.c b/hw/pc.c
index 052c0a2..85e59cc 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1332,8 +1332,7 @@ static void pc_init1(ram_addr_t ram_size,
 
     for(i = 0; i < MAX_PARALLEL_PORTS; i++) {
         if (parallel_hds[i]) {
-            parallel_init(parallel_io[i], isa_reserve_irq(parallel_irq[i]),
-                          parallel_hds[i]);
+            parallel_init(i, parallel_hds[i]);
         }
     }
 
diff --git a/hw/pc.h b/hw/pc.h
index b547d17..5139616 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -17,7 +17,7 @@ SerialState *serial_isa_init(int index, CharDriverState *chr);
 /* parallel.c */
 
 typedef struct ParallelState ParallelState;
-ParallelState *parallel_init(int base, qemu_irq irq, CharDriverState *chr);
+ParallelState *parallel_init(int index, CharDriverState *chr);
 ParallelState *parallel_mm_init(target_phys_addr_t base, int it_shift, qemu_irq irq, CharDriverState *chr);
 
 /* i8259.c */
diff --git a/hw/sun4u.c b/hw/sun4u.c
index 2b55cf3..e93c007 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -593,8 +593,7 @@ static void sun4uv_init(ram_addr_t RAM_size,
 
     for(i = 0; i < MAX_PARALLEL_PORTS; i++) {
         if (parallel_hds[i]) {
-            parallel_init(parallel_io[i], NULL/*parallel_irq[i]*/,
-                          parallel_hds[i]);
+            parallel_init(i, parallel_hds[i]);
         }
     }
 
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 1/5] floppy: add drive properties.

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/fdc.c        |   50 +++++++++++++++++++++++++++++++++++++-------------
>  hw/fdc.h        |    7 ++++---
>  hw/mips_jazz.c  |    5 ++---
>  hw/mips_malta.c |    5 ++---
>  hw/pc.c         |    6 ++----
>  hw/ppc_prep.c   |    6 ++----
>  hw/sun4m.c      |   16 ++++------------
>  hw/sun4u.c      |    6 ++----
>  8 files changed, 55 insertions(+), 46 deletions(-)
>
> diff --git a/hw/fdc.c b/hw/fdc.c
> index 389d9e6..ea3b8ac 100644
> --- a/hw/fdc.c
> +++ b/hw/fdc.c
[...]
> @@ -1829,43 +1830,50 @@ static void fdctrl_result_timer(void *opaque)
>  }
>  
>  /* Init functions */
> -static void fdctrl_connect_drives(fdctrl_t *fdctrl, BlockDriverState **fds)
> +static void fdctrl_connect_drives(fdctrl_t *fdctrl)
>  {
>      unsigned int i;
>  
>      for (i = 0; i < MAX_FD; i++) {
> -        fd_init(&fdctrl->drives[i], fds[i]);
> +        fd_init(&fdctrl->drives[i]);
>          fd_revalidate(&fdctrl->drives[i]);
>      }
>  }
>  
> -fdctrl_t *fdctrl_init_isa(BlockDriverState **fds)
> +fdctrl_t *fdctrl_init_isa(DriveInfo **fds)
>  {
>      fdctrl_t *fdctrl;
>      ISADevice *dev;
>      int dma_chann = 2;
>  
> -    dev = isa_create_simple("isa-fdc");
> +    dev = isa_create("isa-fdc");
> +    qdev_prop_set_drive(&dev->qdev, "driveA", fds[0]);
> +    qdev_prop_set_drive(&dev->qdev, "driveB", fds[1]);
> +    if (qdev_init(&dev->qdev) != 0)
> +        return NULL;

Callers never check for failure.  Recommend qdev_init_nofail(), except
that's not merged, yet.

>      fdctrl = &(DO_UPCAST(fdctrl_isabus_t, busdev, dev)->state);
>  
>      fdctrl->dma_chann = dma_chann;
>      DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
>  
> -    fdctrl_connect_drives(fdctrl, fds);
> +    fdctrl_connect_drives(fdctrl);
>  
>      return fdctrl;
>  }
>  
>  fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
>                               target_phys_addr_t mmio_base,
> -                             BlockDriverState **fds)
> +                             DriveInfo **fds)
>  {
>      fdctrl_t *fdctrl;
>      DeviceState *dev;
>      fdctrl_sysbus_t *sys;
>  
>      dev = qdev_create(NULL, "sysbus-fdc");
> -    qdev_init(dev);
> +    qdev_prop_set_drive(dev, "driveA", fds[0]);
> +    qdev_prop_set_drive(dev, "driveB", fds[1]);
> +    if (qdev_init(dev) != 0)
> +        return NULL;

Ditto.

>      sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
>      fdctrl = &sys->state;
>      sysbus_connect_irq(&sys->busdev, 0, irq);
> @@ -1873,20 +1881,22 @@ fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
>  
>      fdctrl->dma_chann = dma_chann;
>      DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
> -    fdctrl_connect_drives(fdctrl, fds);
> +    fdctrl_connect_drives(fdctrl);
>  
>      return fdctrl;
>  }
>  
>  fdctrl_t *sun4m_fdctrl_init (qemu_irq irq, target_phys_addr_t io_base,
> -                             BlockDriverState **fds, qemu_irq *fdc_tc)
> +                             DriveInfo **fds, qemu_irq *fdc_tc)
>  {
>      DeviceState *dev;
>      fdctrl_sysbus_t *sys;
>      fdctrl_t *fdctrl;
>  
>      dev = qdev_create(NULL, "SUNW,fdtwo");
> -    qdev_init(dev);
> +    qdev_prop_set_drive(dev, "drive", fds[0]);
> +    if (qdev_init(dev) != 0)
> +        return NULL;

Ditto.

>      sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
>      fdctrl = &sys->state;
>      sysbus_connect_irq(&sys->busdev, 0, irq);
[...]

Re: [Qemu-devel] [PATCH 2/5] floppy: move dma setup + drive connect to fdctrl_init_common()

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/fdc.c |   33 ++++++++++++---------------------
>  1 files changed, 12 insertions(+), 21 deletions(-)
>
> diff --git a/hw/fdc.c b/hw/fdc.c
> index ea3b8ac..537db66 100644
> --- a/hw/fdc.c
> +++ b/hw/fdc.c
> @@ -1842,23 +1842,14 @@ static void fdctrl_connect_drives(fdctrl_t *fdctrl)
>  
>  fdctrl_t *fdctrl_init_isa(DriveInfo **fds)
>  {
> -    fdctrl_t *fdctrl;
>      ISADevice *dev;
> -    int dma_chann = 2;
>  
>      dev = isa_create("isa-fdc");
>      qdev_prop_set_drive(&dev->qdev, "driveA", fds[0]);
>      qdev_prop_set_drive(&dev->qdev, "driveB", fds[1]);
>      if (qdev_init(&dev->qdev) != 0)
>          return NULL;
> -    fdctrl = &(DO_UPCAST(fdctrl_isabus_t, busdev, dev)->state);
> -
> -    fdctrl->dma_chann = dma_chann;
> -    DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
> -
> -    fdctrl_connect_drives(fdctrl);
> -
> -    return fdctrl;
> +    return &(DO_UPCAST(fdctrl_isabus_t, busdev, dev)->state);

Redundant parenthesis.

>  }
>  
>  fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
> @@ -1870,19 +1861,16 @@ fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
>      fdctrl_sysbus_t *sys;
>  
>      dev = qdev_create(NULL, "sysbus-fdc");
> +    sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
> +    fdctrl = &sys->state;
> +    fdctrl->dma_chann = dma_chann; /* FIXME */

What needs to be fixed here?  Could that be explained in the comment?

>      qdev_prop_set_drive(dev, "driveA", fds[0]);
>      qdev_prop_set_drive(dev, "driveB", fds[1]);
>      if (qdev_init(dev) != 0)
>          return NULL;
> -    sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
> -    fdctrl = &sys->state;
>      sysbus_connect_irq(&sys->busdev, 0, irq);
>      sysbus_mmio_map(&sys->busdev, 0, mmio_base);
>  
> -    fdctrl->dma_chann = dma_chann;
> -    DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
> -    fdctrl_connect_drives(fdctrl);
> -
>      return fdctrl;
>  }
>  
> @@ -1901,11 +1889,7 @@ fdctrl_t *sun4m_fdctrl_init (qemu_irq irq, target_phys_addr_t io_base,
>      fdctrl = &sys->state;
>      sysbus_connect_irq(&sys->busdev, 0, irq);
>      sysbus_mmio_map(&sys->busdev, 0, io_base);
> -    *fdc_tc = qdev_get_gpio_in(dev, 0);
> -
> -    fdctrl->dma_chann = -1;
> -
> -    fdctrl_connect_drives(fdctrl);
> +    *fdc_tc = qdev_get_gpio_in(dev, 0); /* FIXME */

Same question.

>  
>      return fdctrl;
>  }
> @@ -1937,6 +1921,10 @@ static int fdctrl_init_common(fdctrl_t *fdctrl)
>      fdctrl->config = FD_CONFIG_EIS | FD_CONFIG_EFIFO; /* Implicit seek, polling & FIFO enabled */
>      fdctrl->num_floppies = MAX_FD;
>  
> +    if (fdctrl->dma_chann != -1)
> +        DMA_register_channel(fdctrl->dma_chann, &fdctrl_transfer_handler, fdctrl);
> +    fdctrl_connect_drives(fdctrl);
> +
>      fdctrl_external_reset(fdctrl);
>      vmstate_register(-1, &vmstate_fdc, fdctrl);
>      qemu_register_reset(fdctrl_external_reset, fdctrl);
> @@ -1949,6 +1937,7 @@ static int isabus_fdc_init1(ISADevice *dev)
>      fdctrl_t *fdctrl = &isa->state;
>      int iobase = 0x3f0;
>      int isairq = 6;
> +    int dma_chann = 2;
>  
>      register_ioport_read(iobase + 0x01, 5, 1,
>                           &fdctrl_read_port, fdctrl);
> @@ -1959,6 +1948,7 @@ static int isabus_fdc_init1(ISADevice *dev)
>      register_ioport_write(iobase + 0x07, 1, 1,
>                            &fdctrl_write_port, fdctrl);
>      isa_init_irq(&isa->busdev, &fdctrl->irq, isairq);
> +    fdctrl->dma_chann = dma_chann;
>  
>      return fdctrl_init_common(fdctrl);
>  }
> @@ -1972,6 +1962,7 @@ static int sysbus_fdc_init1(SysBusDevice *dev)
>      sysbus_init_mmio(dev, 0x08, io);
>      sysbus_init_irq(dev, &fdctrl->irq);
>      qdev_init_gpio_in(&dev->qdev, fdctrl_handle_tc, 1);
> +    fdctrl->dma_chann = -1;
>  
>      return fdctrl_init_common(fdctrl);
>  }

Hmm, as far as I can see, initialization of fdctrl->dma_chann moved to
the qdev init() method for ISA and Sun4m, but not for sysbus.
Intentional?  If yes, what about explaining it in the code, or perhaps
the commit message?

Re: [Qemu-devel] [PATCH 2/5] floppy: move dma setup + drive connect to fdctrl_init_common()

[Gerd Hoffmann]

>>   fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
>> @@ -1870,19 +1861,16 @@ fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
>>       fdctrl_sysbus_t *sys;
>>
>>       dev = qdev_create(NULL, "sysbus-fdc");
>> +    sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
>> +    fdctrl =&sys->state;
>> +    fdctrl->dma_chann = dma_chann; /* FIXME */
>
> What needs to be fixed here?  Could that be explained in the comment?

See below.

>> -    fdctrl_connect_drives(fdctrl);
>> +    *fdc_tc = qdev_get_gpio_in(dev, 0); /* FIXME */
>
> Same question.

fdctrl_init_$kind() should just be convinience wrappers.  Creating the 
devices via -device should work too.  Which means the convinience 
wrappers must do nothing but qdev_create + set properties + qdev_init.

> Hmm, as far as I can see, initialization of fdctrl->dma_chann moved to
> the qdev init() method for ISA and Sun4m, but not for sysbus.
> Intentional?  If yes, what about explaining it in the code, or perhaps
> the commit message?

Yes, intentional.  On the ISA bus the dma channel is fixed: #2.  sun4m 
doesn't use dma.  The third variant has one user which uses dma channel 
#0.  So we could either hard-code that one too (like we do for isa).  Or 
we could make it a property so it can be configured as needed.  Dunno 
which of the two variants would be the correct one.

cheers,
   Gerd

Re: [Qemu-devel] [PATCH 4/5] serial: convert isa to qdev

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/mips_malta.c |    4 +-
>  hw/mips_r4k.c   |    6 +---
>  hw/pc.c         |    6 +---
>  hw/pc.h         |    1 +
>  hw/ppc_prep.c   |    3 +-
>  hw/serial.c     |   77 +++++++++++++++++++++++++++++++++++++++++++++++-------
>  hw/sun4u.c      |    6 +---
>  7 files changed, 75 insertions(+), 28 deletions(-)

Stupid question: how did you figure out which serials are ISA?

[...]
> diff --git a/hw/pc.h b/hw/pc.h
> index c9cdd4a..b547d17 100644
> --- a/hw/pc.h
> +++ b/hw/pc.h
> @@ -12,6 +12,7 @@ SerialState *serial_init(int base, qemu_irq irq, int baudbase,
>  SerialState *serial_mm_init (target_phys_addr_t base, int it_shift,
>                               qemu_irq irq, int baudbase,
>                               CharDriverState *chr, int ioregister);
> +SerialState *serial_isa_init(int index, CharDriverState *chr);
>  
>  /* parallel.c */
>  
> diff --git a/hw/ppc_prep.c b/hw/ppc_prep.c
> index 32f987c..3f57cbb 100644
> --- a/hw/ppc_prep.c
> +++ b/hw/ppc_prep.c
> @@ -684,7 +684,8 @@ static void ppc_prep_init (ram_addr_t ram_size,
>      //    pit = pit_init(0x40, i8259[0]);
>      rtc_init(2000);
>  
> -    serial_init(0x3f8, i8259[4], 115200, serial_hds[0]);
> +    if (serial_hds[0])
> +        serial_isa_init(0, serial_hds[0]);
>      nb_nics1 = nb_nics;
>      if (nb_nics1 > NE2000_NB_MAX)
>          nb_nics1 = NE2000_NB_MAX;

Why the new conditional?  Bug fix perhaps?

> diff --git a/hw/serial.c b/hw/serial.c
> index 1f4ce77..81021e9 100644
> --- a/hw/serial.c
> +++ b/hw/serial.c
[...]
> @@ -733,7 +735,37 @@ static void serial_init_core(SerialState *s, qemu_irq irq, int baudbase,
>                            serial_event, s);
>  }
>  
> -/* If fd is zero, it means that the serial device uses the console */
> +static int serial_isa_initfn(ISADevice *dev)
> +{
> +    ISASerialState *isa = DO_UPCAST(ISASerialState, dev, dev);
> +    SerialState *s = &isa->state;
> +
> +    s->baudbase = 115200;

The initial baudbase.  Used to be defined as argument to serial_init(),
but all arguments were 115200.  Okay.

> +    isa_init_irq(dev, &s->irq, isa->isairq);
> +    serial_init_core(s);
> +    vmstate_register(isa->iobase, &vmstate_serial, s);
> +
> +    register_ioport_write(isa->iobase, 8, 1, serial_ioport_write, s);
> +    register_ioport_read(isa->iobase, 8, 1, serial_ioport_read, s);
> +    return 0;
> +}
> +
[...]

Re: [Qemu-devel] [PATCH 5/5] parallel: convert isa to qdev

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/mips_malta.c |    2 +-
>  hw/parallel.c   |   80 ++++++++++++++++++++++++++++++++++++++++++------------
>  hw/pc.c         |    3 +-
>  hw/pc.h         |    2 +-
>  hw/sun4u.c      |    3 +-
>  5 files changed, 66 insertions(+), 24 deletions(-)
>
[...]
> diff --git a/hw/parallel.c b/hw/parallel.c
> index faaaa0d..30c53a2 100644
> --- a/hw/parallel.c
> +++ b/hw/parallel.c
> @@ -78,6 +78,13 @@ struct ParallelState {
>      int it_shift;
>  };
>  
> +typedef struct ISAParallelState {
> +    ISADevice dev;
> +    uint32_t iobase;
> +    uint32_t isairq;
> +    ParallelState state;
> +} ISAParallelState;
> +
>  static void parallel_update_irq(ParallelState *s)
>  {
>      if (s->irq_pending)
> @@ -438,38 +445,56 @@ static void parallel_reset(void *opaque)
>      s->last_read_offset = ~0U;
>  }
>  
> -/* If fd is zero, it means that the parallel device uses the console */
> -ParallelState *parallel_init(int base, qemu_irq irq, CharDriverState *chr)
> +static int parallel_isa_initfn(ISADevice *dev)
>  {
> -    ParallelState *s;
> +    ISAParallelState *isa = DO_UPCAST(ISAParallelState, dev, dev);
> +    ParallelState *s = &isa->state;
>      uint8_t dummy;
>  
> -    s = qemu_mallocz(sizeof(ParallelState));
> -    s->irq = irq;
> -    s->chr = chr;
> +    if (!s->chr) {
> +        fprintf(stderr, "Can't create parallel device, empty char device\n");
> +        exit(1);
> +    }
> +
> +    isa_init_irq(dev, &s->irq, isa->isairq);
>      parallel_reset(s);
>      qemu_register_reset(parallel_reset, s);
>  
> -    if (qemu_chr_ioctl(chr, CHR_IOCTL_PP_READ_STATUS, &dummy) == 0) {
> +    if (qemu_chr_ioctl(s->chr, CHR_IOCTL_PP_READ_STATUS, &dummy) == 0) {
>          s->hw_driver = 1;
>          s->status = dummy;
>      }
>  
>      if (s->hw_driver) {
> -        register_ioport_write(base, 8, 1, parallel_ioport_write_hw, s);
> -        register_ioport_read(base, 8, 1, parallel_ioport_read_hw, s);
> -        register_ioport_write(base+4, 1, 2, parallel_ioport_eppdata_write_hw2, s);
> -        register_ioport_read(base+4, 1, 2, parallel_ioport_eppdata_read_hw2, s);
> -        register_ioport_write(base+4, 1, 4, parallel_ioport_eppdata_write_hw4, s);
> -        register_ioport_read(base+4, 1, 4, parallel_ioport_eppdata_read_hw4, s);
> -        register_ioport_write(base+0x400, 8, 1, parallel_ioport_ecp_write, s);
> -        register_ioport_read(base+0x400, 8, 1, parallel_ioport_ecp_read, s);
> +        register_ioport_write(isa->iobase, 8, 1, parallel_ioport_write_hw, s);
> +        register_ioport_read(isa->iobase, 8, 1, parallel_ioport_read_hw, s);
> +        register_ioport_write(isa->iobase+4, 1, 2, parallel_ioport_eppdata_write_hw2, s);
> +        register_ioport_read(isa->iobase+4, 1, 2, parallel_ioport_eppdata_read_hw2, s);
> +        register_ioport_write(isa->iobase+4, 1, 4, parallel_ioport_eppdata_write_hw4, s);
> +        register_ioport_read(isa->iobase+4, 1, 4, parallel_ioport_eppdata_read_hw4, s);
> +        register_ioport_write(isa->iobase+0x400, 8, 1, parallel_ioport_ecp_write, s);
> +        register_ioport_read(isa->iobase+0x400, 8, 1, parallel_ioport_ecp_read, s);
>      }
>      else {
> -        register_ioport_write(base, 8, 1, parallel_ioport_write_sw, s);
> -        register_ioport_read(base, 8, 1, parallel_ioport_read_sw, s);
> +        register_ioport_write(isa->iobase, 8, 1, parallel_ioport_write_sw, s);
> +        register_ioport_read(isa->iobase, 8, 1, parallel_ioport_read_sw, s);

Suggest to use "int base = isa->iobase" here, to reduce line length.
Also yields a tidier diff.

>      }
> -    return s;
> +    return 0;
> +}
> +
[...]

Re: [Qemu-devel] [PATCH 0/5] isa: more qdev conversions.

[Markus Armbruster]

Gerd Hoffmann <kraxel@redhat.com> writes:

>   Hi,
>
> This patch series converts more isa devices to qdev.
>
> cheers,
>   Gerd

Good stuff.

Re: [Qemu-devel] [PATCH 4/5] serial: convert isa to qdev

[Gerd Hoffmann]

On 09/18/09 17:04, Markus Armbruster wrote:
> Gerd Hoffmann<kraxel@redhat.com>  writes:
>
>> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>> ---
>>   hw/mips_malta.c |    4 +-
>>   hw/mips_r4k.c   |    6 +---
>>   hw/pc.c         |    6 +---
>>   hw/pc.h         |    1 +
>>   hw/ppc_prep.c   |    3 +-
>>   hw/serial.c     |   77 +++++++++++++++++++++++++++++++++++++++++++++++-------
>>   hw/sun4u.c      |    6 +---
>>   7 files changed, 75 insertions(+), 28 deletions(-)
>
> Stupid question: how did you figure out which serials are ISA?

Look at I/O ports + irq windup.

>> +    s->baudbase = 115200;
>
> The initial baudbase.  Used to be defined as argument to serial_init(),
> but all arguments were 115200.  Okay.

Yea.  Was a bit sloppy with commit messages on this series, guess it 
needs a respin just because of that ...

cheers,
   Gerd

[Qemu-devel] [PATCH 2/5] floppy: move dma setup + drive connect to fdctrl_init_common()

[Gerd Hoffmann]

isa-fdc is completely qdev-ified with this patch applied, all
configuration uses properties.

sysbus-fdc needs dma_channel config fixed.  There is only one user
(mips_jazz) which uses dma channel 0.  Can anyone knowing this
platform suggest how to deal with it?  Is hardcoding fine?

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/fdc.c |   31 +++++++++++--------------------
 1 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/hw/fdc.c b/hw/fdc.c
index ea3b8ac..b9c0729 100644
--- a/hw/fdc.c
+++ b/hw/fdc.c
@@ -1842,23 +1842,14 @@ static void fdctrl_connect_drives(fdctrl_t *fdctrl)
 
 fdctrl_t *fdctrl_init_isa(DriveInfo **fds)
 {
-    fdctrl_t *fdctrl;
     ISADevice *dev;
-    int dma_chann = 2;
 
     dev = isa_create("isa-fdc");
     qdev_prop_set_drive(&dev->qdev, "driveA", fds[0]);
     qdev_prop_set_drive(&dev->qdev, "driveB", fds[1]);
     if (qdev_init(&dev->qdev) != 0)
         return NULL;
-    fdctrl = &(DO_UPCAST(fdctrl_isabus_t, busdev, dev)->state);
-
-    fdctrl->dma_chann = dma_chann;
-    DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
-
-    fdctrl_connect_drives(fdctrl);
-
-    return fdctrl;
+    return &(DO_UPCAST(fdctrl_isabus_t, busdev, dev)->state);
 }
 
 fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
@@ -1870,19 +1861,16 @@ fdctrl_t *fdctrl_init_sysbus(qemu_irq irq, int dma_chann,
     fdctrl_sysbus_t *sys;
 
     dev = qdev_create(NULL, "sysbus-fdc");
+    sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
+    fdctrl = &sys->state;
+    fdctrl->dma_chann = dma_chann; /* FIXME */
     qdev_prop_set_drive(dev, "driveA", fds[0]);
     qdev_prop_set_drive(dev, "driveB", fds[1]);
     if (qdev_init(dev) != 0)
         return NULL;
-    sys = DO_UPCAST(fdctrl_sysbus_t, busdev.qdev, dev);
-    fdctrl = &sys->state;
     sysbus_connect_irq(&sys->busdev, 0, irq);
     sysbus_mmio_map(&sys->busdev, 0, mmio_base);
 
-    fdctrl->dma_chann = dma_chann;
-    DMA_register_channel(dma_chann, &fdctrl_transfer_handler, fdctrl);
-    fdctrl_connect_drives(fdctrl);
-
     return fdctrl;
 }
 
@@ -1903,10 +1891,6 @@ fdctrl_t *sun4m_fdctrl_init (qemu_irq irq, target_phys_addr_t io_base,
     sysbus_mmio_map(&sys->busdev, 0, io_base);
     *fdc_tc = qdev_get_gpio_in(dev, 0);
 
-    fdctrl->dma_chann = -1;
-
-    fdctrl_connect_drives(fdctrl);
-
     return fdctrl;
 }
 
@@ -1937,6 +1921,10 @@ static int fdctrl_init_common(fdctrl_t *fdctrl)
     fdctrl->config = FD_CONFIG_EIS | FD_CONFIG_EFIFO; /* Implicit seek, polling & FIFO enabled */
     fdctrl->num_floppies = MAX_FD;
 
+    if (fdctrl->dma_chann != -1)
+        DMA_register_channel(fdctrl->dma_chann, &fdctrl_transfer_handler, fdctrl);
+    fdctrl_connect_drives(fdctrl);
+
     fdctrl_external_reset(fdctrl);
     vmstate_register(-1, &vmstate_fdc, fdctrl);
     qemu_register_reset(fdctrl_external_reset, fdctrl);
@@ -1949,6 +1937,7 @@ static int isabus_fdc_init1(ISADevice *dev)
     fdctrl_t *fdctrl = &isa->state;
     int iobase = 0x3f0;
     int isairq = 6;
+    int dma_chann = 2;
 
     register_ioport_read(iobase + 0x01, 5, 1,
                          &fdctrl_read_port, fdctrl);
@@ -1959,6 +1948,7 @@ static int isabus_fdc_init1(ISADevice *dev)
     register_ioport_write(iobase + 0x07, 1, 1,
                           &fdctrl_write_port, fdctrl);
     isa_init_irq(&isa->busdev, &fdctrl->irq, isairq);
+    fdctrl->dma_chann = dma_chann;
 
     return fdctrl_init_common(fdctrl);
 }
@@ -1972,6 +1962,7 @@ static int sysbus_fdc_init1(SysBusDevice *dev)
     sysbus_init_mmio(dev, 0x08, io);
     sysbus_init_irq(dev, &fdctrl->irq);
     qdev_init_gpio_in(&dev->qdev, fdctrl_handle_tc, 1);
+    fdctrl->dma_chann = -1;
 
     return fdctrl_init_common(fdctrl);
 }
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 2/5] floppy: move dma setup + drive connect to fdctrl_init_common()

[Hervé Poussineau]

Hi,

Gerd Hoffmann a écrit :
> isa-fdc is completely qdev-ified with this patch applied, all
> configuration uses properties.
>
> sysbus-fdc needs dma_channel config fixed.  There is only one user
> (mips_jazz) which uses dma channel 0.  Can anyone knowing this
> platform suggest how to deal with it?  Is hardcoding fine?
>
>   

MIPS Jazz floppy drive should not be connected to ISA DMA, but to Jazz 
DMA. Unfortunately, atm, floppy emulation is too tied to isa dma 
controller to be able to do this (like esp which gets dma read/write 
function pointers)...
Either you can change floppy to handle other dma engines, or you can 
hardcode the dma channel to whatever you want (will be wrong in all 
cases). I simply ask you to keep dma enabled.

Hervé