Patchwork [2/8,MIPS] qdev: convert ds1225y nvram to sysbus device

login
register
mail settings
Submitter Hervé Poussineau
Date Sept. 8, 2010, 8:39 p.m.
Message ID <1283978392-6313-3-git-send-email-hpoussin@reactos.org>
Download mbox | patch
Permalink /patch/64206/
State New
Headers show

Comments

Hervé Poussineau - Sept. 8, 2010, 8:39 p.m.
Use it in Jazz emulation
Remove protection stuff, which doesn't belong to this device
Remove ds1225y_init() and ds1225y_set_protection() functions, which are not used anymore

Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/ds1225y.c   |  151 ++++++++++++++++++++++++++++++--------------------------
 hw/mips.h      |    4 --
 hw/mips_jazz.c |    4 +-
 3 files changed, 83 insertions(+), 76 deletions(-)
Blue Swirl - Sept. 9, 2010, 2:21 p.m.
2010/9/8 Hervé Poussineau <hpoussin@reactos.org>:
> Use it in Jazz emulation
> Remove protection stuff, which doesn't belong to this device
> Remove ds1225y_init() and ds1225y_set_protection() functions, which are not used anymore
>
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/ds1225y.c   |  151 ++++++++++++++++++++++++++++++--------------------------
>  hw/mips.h      |    4 --
>  hw/mips_jazz.c |    4 +-
>  3 files changed, 83 insertions(+), 76 deletions(-)
>
> diff --git a/hw/ds1225y.c b/hw/ds1225y.c
> index 009d127..046d1ec 100644
> --- a/hw/ds1225y.c
> +++ b/hw/ds1225y.c
> @@ -22,31 +22,34 @@
>  * THE SOFTWARE.
>  */
>
> -#include "hw.h"
> -#include "mips.h"
> -#include "nvram.h"
> +#include "sysbus.h"
>
>  //#define DEBUG_NVRAM
>
> -typedef struct ds1225y_t
> -{
> -    uint32_t chip_size;
> +#ifdef DEBUG_NVRAM
> +#define DPRINTF(fmt, ...)                                       \
> +    do { printf("nvram: " fmt , ## __VA_ARGS__); } while (0)
> +#else
> +#define DPRINTF(fmt, ...) do {} while (0)
> +#endif
> +
> +typedef struct {
> +    DeviceState qdev;
> +    int32_t chip_size;
> +    char* filename;
>     QEMUFile *file;
>     uint8_t *contents;
> -    uint8_t protection;
> -} ds1225y_t;
> -
> +} NvRamState;
>
>  static uint32_t nvram_readb (void *opaque, target_phys_addr_t addr)
>  {
> -    ds1225y_t *s = opaque;
> +    NvRamState *s = opaque;
>     uint32_t val;
>
>     val = s->contents[addr];
>
> -#ifdef DEBUG_NVRAM
> -    printf("nvram: read 0x%x at " TARGET_FMT_lx "\n", val, addr);
> -#endif
> +    DPRINTF("read 0x%x at " TARGET_FMT_plx "\n", val, addr);
> +
>     return val;
>  }
>
> @@ -70,11 +73,9 @@ static uint32_t nvram_readl (void *opaque, target_phys_addr_t addr)
>
>  static void nvram_writeb (void *opaque, target_phys_addr_t addr, uint32_t val)
>  {
> -    ds1225y_t *s = opaque;
> +    NvRamState *s = opaque;
>
> -#ifdef DEBUG_NVRAM
> -    printf("nvram: write 0x%x at " TARGET_FMT_lx "\n", val, addr);
> -#endif
> +    DPRINTF("write 0x%x at " TARGET_FMT_plx "\n", val, addr);
>
>     s->contents[addr] = val & 0xff;
>     if (s->file) {
> @@ -98,34 +99,6 @@ static void nvram_writel (void *opaque, target_phys_addr_t addr, uint32_t val)
>     nvram_writeb(opaque, addr + 3, (val >> 24) & 0xff);
>  }
>
> -static void nvram_writeb_protected (void *opaque, target_phys_addr_t addr, uint32_t val)
> -{
> -    ds1225y_t *s = opaque;
> -
> -    if (s->protection != 7) {
> -#ifdef DEBUG_NVRAM
> -    printf("nvram: prevent write of 0x%x at " TARGET_FMT_lx "\n", val, addr);
> -#endif
> -        return;
> -    }
> -
> -    nvram_writeb(opaque, addr, val);
> -}
> -
> -static void nvram_writew_protected (void *opaque, target_phys_addr_t addr, uint32_t val)
> -{
> -    nvram_writeb_protected(opaque, addr, val & 0xff);
> -    nvram_writeb_protected(opaque, addr + 1, (val >> 8) & 0xff);
> -}
> -
> -static void nvram_writel_protected (void *opaque, target_phys_addr_t addr, uint32_t val)
> -{
> -    nvram_writeb_protected(opaque, addr, val & 0xff);
> -    nvram_writeb_protected(opaque, addr + 1, (val >> 8) & 0xff);
> -    nvram_writeb_protected(opaque, addr + 2, (val >> 16) & 0xff);
> -    nvram_writeb_protected(opaque, addr + 3, (val >> 24) & 0xff);
> -}
> -
>  static CPUReadMemoryFunc * const nvram_read[] = {
>     &nvram_readb,
>     &nvram_readw,
> @@ -138,43 +111,81 @@ static CPUWriteMemoryFunc * const nvram_write[] = {
>     &nvram_writel,
>  };
>
> -static CPUWriteMemoryFunc * const nvram_write_protected[] = {
> -    &nvram_writeb_protected,
> -    &nvram_writew_protected,
> -    &nvram_writel_protected,
> +static int nvram_post_load(void *opaque, int version_id)
> +{
> +    NvRamState *s = opaque;
> +
> +    if (s->file) {
> +        qemu_fclose(s->file);
> +    }
> +
> +    /* Write back nvram contents */
> +    s->file = qemu_fopen(s->filename, "wb");

How about ftruncate(fileno(s->file), 0)?

> +    if (s->file) {
> +        /* Write back contents, as 'wb' mode cleaned the file */
> +        qemu_put_buffer(s->file, s->contents, s->chip_size);
> +        qemu_fflush(s->file);
> +    }
> +
> +    return 0;
> +}
> +
> +static const VMStateDescription vmstate_nvram = {
> +    .name = "nvram",
> +    .version_id = 0,
> +    .minimum_version_id = 0,
> +    .minimum_version_id_old = 0,
> +    .post_load = nvram_post_load,
> +    .fields = (VMStateField []) {
> +        VMSTATE_VARRAY_INT32(contents, NvRamState, chip_size, 0, vmstate_info_uint8,
> +                             uint8_t),
> +        VMSTATE_END_OF_LIST()
> +    }
>  };
>
> -/* Initialisation routine */
> -void *ds1225y_init(target_phys_addr_t mem_base, const char *filename)
> +typedef struct {
> +    SysBusDevice busdev;
> +    NvRamState nvram;
> +} SysBusNvRamState;
> +
> +static int nvram_sysbus_initfn(SysBusDevice *dev)
>  {
> -    ds1225y_t *s;
> -    int mem_indexRW, mem_indexRP;
> +    NvRamState *s = &FROM_SYSBUS(SysBusNvRamState, dev)->nvram;
>     QEMUFile *file;
> +    int s_io;
>
> -    s = qemu_mallocz(sizeof(ds1225y_t));
> -    s->chip_size = 0x2000; /* Fixed for ds1225y chip: 8 KiB */
>     s->contents = qemu_mallocz(s->chip_size);
> -    s->protection = 7;
> +
> +    s_io = cpu_register_io_memory(nvram_read, nvram_write, s);
> +    sysbus_init_mmio(dev, s->chip_size, s_io);
>
>     /* Read current file */
> -    file = qemu_fopen(filename, "rb");
> +    file = qemu_fopen(s->filename, "rb");
>     if (file) {
>         /* Read nvram contents */
>         qemu_get_buffer(file, s->contents, s->chip_size);
>         qemu_fclose(file);
>     }
> -    s->file = qemu_fopen(filename, "wb");
> -    if (s->file) {
> -        /* Write back contents, as 'wb' mode cleaned the file */
> -        qemu_put_buffer(s->file, s->contents, s->chip_size);
> -        qemu_fflush(s->file);
> -    }
> +    nvram_post_load(s, 0);
>
> -    /* Read/write memory */
> -    mem_indexRW = cpu_register_io_memory(nvram_read, nvram_write, s);
> -    cpu_register_physical_memory(mem_base, s->chip_size, mem_indexRW);
> -    /* Read/write protected memory */
> -    mem_indexRP = cpu_register_io_memory(nvram_read, nvram_write_protected, s);
> -    cpu_register_physical_memory(mem_base + s->chip_size, s->chip_size, mem_indexRP);
> -    return s;
> +    return 0;
>  }
> +
> +static SysBusDeviceInfo nvram_sysbus_info = {
> +    .qdev.name  = "nvram",
> +    .qdev.size  = sizeof(SysBusNvRamState),
> +    .qdev.vmsd  = &vmstate_nvram,
> +    .init       = nvram_sysbus_initfn,
> +    .qdev.props = (Property[]) {
> +        DEFINE_PROP_INT32("size", SysBusNvRamState, nvram.chip_size, 0x2000),
> +        DEFINE_PROP_STRING("filename", SysBusNvRamState, nvram.filename),
> +        DEFINE_PROP_END_OF_LIST(),
> +    },
> +};
> +
> +static void nvram_register(void)
> +{
> +    sysbus_register_withprop(&nvram_sysbus_info);
> +}
> +
> +device_init(nvram_register)
> diff --git a/hw/mips.h b/hw/mips.h
> index 617ea10..8f32ba0 100644
> --- a/hw/mips.h
> +++ b/hw/mips.h
> @@ -8,10 +8,6 @@ PCIBus *pci_gt64120_init(qemu_irq *pic);
>  /* bonito.c */
>  PCIBus *bonito_init(qemu_irq *pic);
>
> -/* ds1225y.c */
> -void *ds1225y_init(target_phys_addr_t mem_base, const char *filename);
> -void ds1225y_set_protection(void *opaque, int protection);
> -
>  /* g364fb.c */
>  int g364fb_mm_init(target_phys_addr_t vram_base,
>                    target_phys_addr_t ctrl_base, int it_shift,
> diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
> index e306839..06968d3 100644
> --- a/hw/mips_jazz.c
> +++ b/hw/mips_jazz.c
> @@ -290,8 +290,8 @@ void mips_jazz_init (ram_addr_t ram_size,
>     /* FIXME: missing Jazz sound at 0x8000c000, rc4030[2] */
>     audio_init(i8259);
>
> -    /* NVRAM: Unprotected at 0x9000, Protected at 0xa000, Read only at 0xb000 */
> -    ds1225y_init(0x80009000, "nvram");
> +    /* NVRAM */
> +    sysbus_create_simple("nvram", 0x80009000, NULL);
>
>     /* LED indicator */
>     jazz_led_init(0x8000f000);
> --
> 1.7.1.GIT
>
>
>
Markus Armbruster - Sept. 25, 2010, 9:52 a.m.
Hervé Poussineau <hpoussin@reactos.org> writes:

> Use it in Jazz emulation
> Remove protection stuff, which doesn't belong to this device
> Remove ds1225y_init() and ds1225y_set_protection() functions, which are not used anymore
>
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>

I think this would be easier to review if you split it up some, perhaps
cleanup such as DPRINTF(), removal of protection stuff, and qdev
conversion.

> ---
>  hw/ds1225y.c   |  151 ++++++++++++++++++++++++++++++--------------------------
>  hw/mips.h      |    4 --
>  hw/mips_jazz.c |    4 +-
>  3 files changed, 83 insertions(+), 76 deletions(-)
>
> diff --git a/hw/ds1225y.c b/hw/ds1225y.c
> index 009d127..046d1ec 100644
> --- a/hw/ds1225y.c
> +++ b/hw/ds1225y.c
> @@ -22,31 +22,34 @@
>   * THE SOFTWARE.
>   */
>  
> -#include "hw.h"
> -#include "mips.h"
> -#include "nvram.h"
> +#include "sysbus.h"
>  
>  //#define DEBUG_NVRAM
>  
> -typedef struct ds1225y_t
> -{
> -    uint32_t chip_size;
> +#ifdef DEBUG_NVRAM
> +#define DPRINTF(fmt, ...)                                       \
> +    do { printf("nvram: " fmt , ## __VA_ARGS__); } while (0)
> +#else
> +#define DPRINTF(fmt, ...) do {} while (0)
> +#endif
> +
> +typedef struct {
> +    DeviceState qdev;
> +    int32_t chip_size;

You change chip_size from unsigned to signed.  Why?

> +    char* filename;
>      QEMUFile *file;
>      uint8_t *contents;
> -    uint8_t protection;
> -} ds1225y_t;
> -
> +} NvRamState;
[...]

Patch

diff --git a/hw/ds1225y.c b/hw/ds1225y.c
index 009d127..046d1ec 100644
--- a/hw/ds1225y.c
+++ b/hw/ds1225y.c
@@ -22,31 +22,34 @@ 
  * THE SOFTWARE.
  */
 
-#include "hw.h"
-#include "mips.h"
-#include "nvram.h"
+#include "sysbus.h"
 
 //#define DEBUG_NVRAM
 
-typedef struct ds1225y_t
-{
-    uint32_t chip_size;
+#ifdef DEBUG_NVRAM
+#define DPRINTF(fmt, ...)                                       \
+    do { printf("nvram: " fmt , ## __VA_ARGS__); } while (0)
+#else
+#define DPRINTF(fmt, ...) do {} while (0)
+#endif
+
+typedef struct {
+    DeviceState qdev;
+    int32_t chip_size;
+    char* filename;
     QEMUFile *file;
     uint8_t *contents;
-    uint8_t protection;
-} ds1225y_t;
-
+} NvRamState;
 
 static uint32_t nvram_readb (void *opaque, target_phys_addr_t addr)
 {
-    ds1225y_t *s = opaque;
+    NvRamState *s = opaque;
     uint32_t val;
 
     val = s->contents[addr];
 
-#ifdef DEBUG_NVRAM
-    printf("nvram: read 0x%x at " TARGET_FMT_lx "\n", val, addr);
-#endif
+    DPRINTF("read 0x%x at " TARGET_FMT_plx "\n", val, addr);
+
     return val;
 }
 
@@ -70,11 +73,9 @@  static uint32_t nvram_readl (void *opaque, target_phys_addr_t addr)
 
 static void nvram_writeb (void *opaque, target_phys_addr_t addr, uint32_t val)
 {
-    ds1225y_t *s = opaque;
+    NvRamState *s = opaque;
 
-#ifdef DEBUG_NVRAM
-    printf("nvram: write 0x%x at " TARGET_FMT_lx "\n", val, addr);
-#endif
+    DPRINTF("write 0x%x at " TARGET_FMT_plx "\n", val, addr);
 
     s->contents[addr] = val & 0xff;
     if (s->file) {
@@ -98,34 +99,6 @@  static void nvram_writel (void *opaque, target_phys_addr_t addr, uint32_t val)
     nvram_writeb(opaque, addr + 3, (val >> 24) & 0xff);
 }
 
-static void nvram_writeb_protected (void *opaque, target_phys_addr_t addr, uint32_t val)
-{
-    ds1225y_t *s = opaque;
-
-    if (s->protection != 7) {
-#ifdef DEBUG_NVRAM
-    printf("nvram: prevent write of 0x%x at " TARGET_FMT_lx "\n", val, addr);
-#endif
-        return;
-    }
-
-    nvram_writeb(opaque, addr, val);
-}
-
-static void nvram_writew_protected (void *opaque, target_phys_addr_t addr, uint32_t val)
-{
-    nvram_writeb_protected(opaque, addr, val & 0xff);
-    nvram_writeb_protected(opaque, addr + 1, (val >> 8) & 0xff);
-}
-
-static void nvram_writel_protected (void *opaque, target_phys_addr_t addr, uint32_t val)
-{
-    nvram_writeb_protected(opaque, addr, val & 0xff);
-    nvram_writeb_protected(opaque, addr + 1, (val >> 8) & 0xff);
-    nvram_writeb_protected(opaque, addr + 2, (val >> 16) & 0xff);
-    nvram_writeb_protected(opaque, addr + 3, (val >> 24) & 0xff);
-}
-
 static CPUReadMemoryFunc * const nvram_read[] = {
     &nvram_readb,
     &nvram_readw,
@@ -138,43 +111,81 @@  static CPUWriteMemoryFunc * const nvram_write[] = {
     &nvram_writel,
 };
 
-static CPUWriteMemoryFunc * const nvram_write_protected[] = {
-    &nvram_writeb_protected,
-    &nvram_writew_protected,
-    &nvram_writel_protected,
+static int nvram_post_load(void *opaque, int version_id)
+{
+    NvRamState *s = opaque;
+
+    if (s->file) {
+        qemu_fclose(s->file);
+    }
+
+    /* Write back nvram contents */
+    s->file = qemu_fopen(s->filename, "wb");
+    if (s->file) {
+        /* Write back contents, as 'wb' mode cleaned the file */
+        qemu_put_buffer(s->file, s->contents, s->chip_size);
+        qemu_fflush(s->file);
+    }
+
+    return 0;
+}
+
+static const VMStateDescription vmstate_nvram = {
+    .name = "nvram",
+    .version_id = 0,
+    .minimum_version_id = 0,
+    .minimum_version_id_old = 0,
+    .post_load = nvram_post_load,
+    .fields = (VMStateField []) {
+        VMSTATE_VARRAY_INT32(contents, NvRamState, chip_size, 0, vmstate_info_uint8,
+                             uint8_t),
+        VMSTATE_END_OF_LIST()
+    }
 };
 
-/* Initialisation routine */
-void *ds1225y_init(target_phys_addr_t mem_base, const char *filename)
+typedef struct {
+    SysBusDevice busdev;
+    NvRamState nvram;
+} SysBusNvRamState;
+
+static int nvram_sysbus_initfn(SysBusDevice *dev)
 {
-    ds1225y_t *s;
-    int mem_indexRW, mem_indexRP;
+    NvRamState *s = &FROM_SYSBUS(SysBusNvRamState, dev)->nvram;
     QEMUFile *file;
+    int s_io;
 
-    s = qemu_mallocz(sizeof(ds1225y_t));
-    s->chip_size = 0x2000; /* Fixed for ds1225y chip: 8 KiB */
     s->contents = qemu_mallocz(s->chip_size);
-    s->protection = 7;
+
+    s_io = cpu_register_io_memory(nvram_read, nvram_write, s);
+    sysbus_init_mmio(dev, s->chip_size, s_io);
 
     /* Read current file */
-    file = qemu_fopen(filename, "rb");
+    file = qemu_fopen(s->filename, "rb");
     if (file) {
         /* Read nvram contents */
         qemu_get_buffer(file, s->contents, s->chip_size);
         qemu_fclose(file);
     }
-    s->file = qemu_fopen(filename, "wb");
-    if (s->file) {
-        /* Write back contents, as 'wb' mode cleaned the file */
-        qemu_put_buffer(s->file, s->contents, s->chip_size);
-        qemu_fflush(s->file);
-    }
+    nvram_post_load(s, 0);
 
-    /* Read/write memory */
-    mem_indexRW = cpu_register_io_memory(nvram_read, nvram_write, s);
-    cpu_register_physical_memory(mem_base, s->chip_size, mem_indexRW);
-    /* Read/write protected memory */
-    mem_indexRP = cpu_register_io_memory(nvram_read, nvram_write_protected, s);
-    cpu_register_physical_memory(mem_base + s->chip_size, s->chip_size, mem_indexRP);
-    return s;
+    return 0;
 }
+
+static SysBusDeviceInfo nvram_sysbus_info = {
+    .qdev.name  = "nvram",
+    .qdev.size  = sizeof(SysBusNvRamState),
+    .qdev.vmsd  = &vmstate_nvram,
+    .init       = nvram_sysbus_initfn,
+    .qdev.props = (Property[]) {
+        DEFINE_PROP_INT32("size", SysBusNvRamState, nvram.chip_size, 0x2000),
+        DEFINE_PROP_STRING("filename", SysBusNvRamState, nvram.filename),
+        DEFINE_PROP_END_OF_LIST(),
+    },
+};
+
+static void nvram_register(void)
+{
+    sysbus_register_withprop(&nvram_sysbus_info);
+}
+
+device_init(nvram_register)
diff --git a/hw/mips.h b/hw/mips.h
index 617ea10..8f32ba0 100644
--- a/hw/mips.h
+++ b/hw/mips.h
@@ -8,10 +8,6 @@  PCIBus *pci_gt64120_init(qemu_irq *pic);
 /* bonito.c */
 PCIBus *bonito_init(qemu_irq *pic);
 
-/* ds1225y.c */
-void *ds1225y_init(target_phys_addr_t mem_base, const char *filename);
-void ds1225y_set_protection(void *opaque, int protection);
-
 /* g364fb.c */
 int g364fb_mm_init(target_phys_addr_t vram_base,
                    target_phys_addr_t ctrl_base, int it_shift,
diff --git a/hw/mips_jazz.c b/hw/mips_jazz.c
index e306839..06968d3 100644
--- a/hw/mips_jazz.c
+++ b/hw/mips_jazz.c
@@ -290,8 +290,8 @@  void mips_jazz_init (ram_addr_t ram_size,
     /* FIXME: missing Jazz sound at 0x8000c000, rc4030[2] */
     audio_init(i8259);
 
-    /* NVRAM: Unprotected at 0x9000, Protected at 0xa000, Read only at 0xb000 */
-    ds1225y_init(0x80009000, "nvram");
+    /* NVRAM */
+    sysbus_create_simple("nvram", 0x80009000, NULL);
 
     /* LED indicator */
     jazz_led_init(0x8000f000);