All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 0/5] vmstateify a bunch of rare devices [for 2.8]
@ 2016-08-24 10:40 Dr. David Alan Gilbert (git)
  2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 1/5] vmstateify ssd0323 display Dr. David Alan Gilbert (git)
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2016-08-24 10:40 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, hpoussin; +Cc: amit.shah, quintela

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

In an effort to nibble away at register_savevm users,
this set removes 5 more and converts them to vmstate.

The ssd0323 seems to test OK with Peter's Stellaris test.
The rc4030 has a tested-by from Hervé.
The other 3 I'm less sure of; the ssi-sd is similar to the ssd0323;
the two tsc2xxx devices I've not got a test for (or at least a test
that works prior to the changes).

(This is v2 since I previously posted the rc4030 and ssd0323 back in July;
but it seems to make sense to roll this lot together)

Dave

Dr. David Alan Gilbert (5):
  vmstateify ssd0323 display
  vmstateify rc4030
  vmstateify ssi-sd
  vmstateify tsc2005
  vmstateify tsc210x

 hw/display/ssd0323.c | 102 +++++++++++---------------
 hw/dma/rc4030.c      |  81 +++++++--------------
 hw/input/tsc2005.c   | 154 +++++++++++++++-------------------------
 hw/input/tsc210x.c   | 197 +++++++++++++++++++++++----------------------------
 hw/sd/ssi-sd.c       |  70 ++++++++----------
 5 files changed, 241 insertions(+), 363 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH v2 1/5] vmstateify ssd0323 display
  2016-08-24 10:40 [Qemu-devel] [PATCH v2 0/5] vmstateify a bunch of rare devices [for 2.8] Dr. David Alan Gilbert (git)
@ 2016-08-24 10:40 ` Dr. David Alan Gilbert (git)
  2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 2/5] vmstateify rc4030 Dr. David Alan Gilbert (git)
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2016-08-24 10:40 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, hpoussin; +Cc: amit.shah, quintela

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

   Bumps version number because we now use the VMSTATE_SSI_SLAVE that
only uses a byte rather than a 32bit (for saving a bool 'cs').

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/display/ssd0323.c | 102 ++++++++++++++++++++-------------------------------
 1 file changed, 40 insertions(+), 62 deletions(-)

diff --git a/hw/display/ssd0323.c b/hw/display/ssd0323.c
index 6d1faf4..e182893 100644
--- a/hw/display/ssd0323.c
+++ b/hw/display/ssd0323.c
@@ -48,18 +48,18 @@ typedef struct {
     SSISlave ssidev;
     QemuConsole *con;
 
-    int cmd_len;
-    int cmd;
-    int cmd_data[8];
-    int row;
-    int row_start;
-    int row_end;
-    int col;
-    int col_start;
-    int col_end;
-    int redraw;
-    int remap;
-    enum ssd0323_mode mode;
+    uint32_t cmd_len;
+    int32_t cmd;
+    int32_t cmd_data[8];
+    int32_t row;
+    int32_t row_start;
+    int32_t row_end;
+    int32_t col;
+    int32_t col_start;
+    int32_t col_end;
+    int32_t redraw;
+    int32_t remap;
+    uint32_t mode;
     uint8_t framebuffer[128 * 80 / 2];
 } ssd0323_state;
 
@@ -279,83 +279,62 @@ static void ssd0323_cd(void *opaque, int n, int level)
     s->mode = level ? SSD0323_DATA : SSD0323_CMD;
 }
 
-static void ssd0323_save(QEMUFile *f, void *opaque)
+static int ssd0323_post_load(void *opaque, int version_id)
 {
-    SSISlave *ss = SSI_SLAVE(opaque);
     ssd0323_state *s = (ssd0323_state *)opaque;
-    int i;
-
-    qemu_put_be32(f, s->cmd_len);
-    qemu_put_be32(f, s->cmd);
-    for (i = 0; i < 8; i++)
-        qemu_put_be32(f, s->cmd_data[i]);
-    qemu_put_be32(f, s->row);
-    qemu_put_be32(f, s->row_start);
-    qemu_put_be32(f, s->row_end);
-    qemu_put_be32(f, s->col);
-    qemu_put_be32(f, s->col_start);
-    qemu_put_be32(f, s->col_end);
-    qemu_put_be32(f, s->redraw);
-    qemu_put_be32(f, s->remap);
-    qemu_put_be32(f, s->mode);
-    qemu_put_buffer(f, s->framebuffer, sizeof(s->framebuffer));
-
-    qemu_put_be32(f, ss->cs);
-}
-
-static int ssd0323_load(QEMUFile *f, void *opaque, int version_id)
-{
-    SSISlave *ss = SSI_SLAVE(opaque);
-    ssd0323_state *s = (ssd0323_state *)opaque;
-    int i;
 
-    if (version_id != 1)
-        return -EINVAL;
-
-    s->cmd_len = qemu_get_be32(f);
-    if (s->cmd_len < 0 || s->cmd_len > ARRAY_SIZE(s->cmd_data)) {
+    if (s->cmd_len > ARRAY_SIZE(s->cmd_data)) {
         return -EINVAL;
     }
-    s->cmd = qemu_get_be32(f);
-    for (i = 0; i < 8; i++)
-        s->cmd_data[i] = qemu_get_be32(f);
-    s->row = qemu_get_be32(f);
     if (s->row < 0 || s->row >= 80) {
         return -EINVAL;
     }
-    s->row_start = qemu_get_be32(f);
     if (s->row_start < 0 || s->row_start >= 80) {
         return -EINVAL;
     }
-    s->row_end = qemu_get_be32(f);
     if (s->row_end < 0 || s->row_end >= 80) {
         return -EINVAL;
     }
-    s->col = qemu_get_be32(f);
     if (s->col < 0 || s->col >= 64) {
         return -EINVAL;
     }
-    s->col_start = qemu_get_be32(f);
     if (s->col_start < 0 || s->col_start >= 64) {
         return -EINVAL;
     }
-    s->col_end = qemu_get_be32(f);
     if (s->col_end < 0 || s->col_end >= 64) {
         return -EINVAL;
     }
-    s->redraw = qemu_get_be32(f);
-    s->remap = qemu_get_be32(f);
-    s->mode = qemu_get_be32(f);
     if (s->mode != SSD0323_CMD && s->mode != SSD0323_DATA) {
         return -EINVAL;
     }
-    qemu_get_buffer(f, s->framebuffer, sizeof(s->framebuffer));
-
-    ss->cs = qemu_get_be32(f);
 
     return 0;
 }
 
+static const VMStateDescription vmstate_ssd0323 = {
+    .name = "ssd0323_oled",
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .post_load = ssd0323_post_load,
+    .fields      = (VMStateField []) {
+        VMSTATE_UINT32(cmd_len, ssd0323_state),
+        VMSTATE_INT32(cmd, ssd0323_state),
+        VMSTATE_INT32_ARRAY(cmd_data, ssd0323_state, 8),
+        VMSTATE_INT32(row, ssd0323_state),
+        VMSTATE_INT32(row_start, ssd0323_state),
+        VMSTATE_INT32(row_end, ssd0323_state),
+        VMSTATE_INT32(col, ssd0323_state),
+        VMSTATE_INT32(col_start, ssd0323_state),
+        VMSTATE_INT32(col_end, ssd0323_state),
+        VMSTATE_INT32(redraw, ssd0323_state),
+        VMSTATE_INT32(remap, ssd0323_state),
+        VMSTATE_UINT32(mode, ssd0323_state),
+        VMSTATE_BUFFER(framebuffer, ssd0323_state),
+        VMSTATE_SSI_SLAVE(ssidev, ssd0323_state),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static const GraphicHwOps ssd0323_ops = {
     .invalidate  = ssd0323_invalidate_display,
     .gfx_update  = ssd0323_update_display,
@@ -372,18 +351,17 @@ static void ssd0323_realize(SSISlave *d, Error **errp)
     qemu_console_resize(s->con, 128 * MAGNIFY, 64 * MAGNIFY);
 
     qdev_init_gpio_in(dev, ssd0323_cd, 1);
-
-    register_savevm(dev, "ssd0323_oled", -1, 1,
-                    ssd0323_save, ssd0323_load, s);
 }
 
 static void ssd0323_class_init(ObjectClass *klass, void *data)
 {
+    DeviceClass *dc = DEVICE_CLASS(klass);
     SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
 
     k->realize = ssd0323_realize;
     k->transfer = ssd0323_transfer;
     k->cs_polarity = SSI_CS_HIGH;
+    dc->vmsd = &vmstate_ssd0323;
 }
 
 static const TypeInfo ssd0323_info = {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH v2 2/5] vmstateify rc4030
  2016-08-24 10:40 [Qemu-devel] [PATCH v2 0/5] vmstateify a bunch of rare devices [for 2.8] Dr. David Alan Gilbert (git)
  2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 1/5] vmstateify ssd0323 display Dr. David Alan Gilbert (git)
@ 2016-08-24 10:40 ` Dr. David Alan Gilbert (git)
  2016-09-26 12:05   ` Dr. David Alan Gilbert
  2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 3/5] vmstateify ssi-sd Dr. David Alan Gilbert (git)
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2016-08-24 10:40 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, hpoussin; +Cc: amit.shah, quintela

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

rc4030 seems to be part of a MIPS chipset; this converts it to
VMState.

  Note:
    a) It builds but I've not found a way to boot a MIPS Jazz image to
test it.
    b) It was saving 0..<15 on the 16 entry rem_speed array; I've not
got a clue what that array is but I'm now saving the whole 16 entries
rather than 15.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>
Tested-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/dma/rc4030.c | 81 +++++++++++++++++++--------------------------------------
 1 file changed, 27 insertions(+), 54 deletions(-)

diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
index 2f2576f..17c8518 100644
--- a/hw/dma/rc4030.c
+++ b/hw/dma/rc4030.c
@@ -616,34 +616,9 @@ static void rc4030_reset(DeviceState *dev)
     qemu_irq_lower(s->jazz_bus_irq);
 }
 
-static int rc4030_load(QEMUFile *f, void *opaque, int version_id)
+static int rc4030_post_load(void *opaque, int version_id)
 {
     rc4030State* s = opaque;
-    int i, j;
-
-    if (version_id != 2)
-        return -EINVAL;
-
-    s->config = qemu_get_be32(f);
-    s->invalid_address_register = qemu_get_be32(f);
-    for (i = 0; i < 8; i++)
-        for (j = 0; j < 4; j++)
-            s->dma_regs[i][j] = qemu_get_be32(f);
-    s->dma_tl_base = qemu_get_be32(f);
-    s->dma_tl_limit = qemu_get_be32(f);
-    s->cache_maint = qemu_get_be32(f);
-    s->remote_failed_address = qemu_get_be32(f);
-    s->memory_failed_address = qemu_get_be32(f);
-    s->cache_ptag = qemu_get_be32(f);
-    s->cache_ltag = qemu_get_be32(f);
-    s->cache_bmask = qemu_get_be32(f);
-    s->memory_refresh_rate = qemu_get_be32(f);
-    s->nvram_protect = qemu_get_be32(f);
-    for (i = 0; i < 15; i++)
-        s->rem_speed[i] = qemu_get_be32(f);
-    s->imr_jazz = qemu_get_be32(f);
-    s->isr_jazz = qemu_get_be32(f);
-    s->itr = qemu_get_be32(f);
 
     set_next_tick(s);
     update_jazz_irq(s);
@@ -651,32 +626,31 @@ static int rc4030_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
-static void rc4030_save(QEMUFile *f, void *opaque)
-{
-    rc4030State* s = opaque;
-    int i, j;
-
-    qemu_put_be32(f, s->config);
-    qemu_put_be32(f, s->invalid_address_register);
-    for (i = 0; i < 8; i++)
-        for (j = 0; j < 4; j++)
-            qemu_put_be32(f, s->dma_regs[i][j]);
-    qemu_put_be32(f, s->dma_tl_base);
-    qemu_put_be32(f, s->dma_tl_limit);
-    qemu_put_be32(f, s->cache_maint);
-    qemu_put_be32(f, s->remote_failed_address);
-    qemu_put_be32(f, s->memory_failed_address);
-    qemu_put_be32(f, s->cache_ptag);
-    qemu_put_be32(f, s->cache_ltag);
-    qemu_put_be32(f, s->cache_bmask);
-    qemu_put_be32(f, s->memory_refresh_rate);
-    qemu_put_be32(f, s->nvram_protect);
-    for (i = 0; i < 15; i++)
-        qemu_put_be32(f, s->rem_speed[i]);
-    qemu_put_be32(f, s->imr_jazz);
-    qemu_put_be32(f, s->isr_jazz);
-    qemu_put_be32(f, s->itr);
-}
+static const VMStateDescription vmstate_rc4030 = {
+    .name = "rc4030",
+    .version_id = 3,
+    .post_load = rc4030_post_load,
+    .fields = (VMStateField []) {
+        VMSTATE_UINT32(config, rc4030State),
+        VMSTATE_UINT32(invalid_address_register, rc4030State),
+        VMSTATE_UINT32_2DARRAY(dma_regs, rc4030State, 8, 4),
+        VMSTATE_UINT32(dma_tl_base, rc4030State),
+        VMSTATE_UINT32(dma_tl_limit, rc4030State),
+        VMSTATE_UINT32(cache_maint, rc4030State),
+        VMSTATE_UINT32(remote_failed_address, rc4030State),
+        VMSTATE_UINT32(memory_failed_address, rc4030State),
+        VMSTATE_UINT32(cache_ptag, rc4030State),
+        VMSTATE_UINT32(cache_ltag, rc4030State),
+        VMSTATE_UINT32(cache_bmask, rc4030State),
+        VMSTATE_UINT32(memory_refresh_rate, rc4030State),
+        VMSTATE_UINT32(nvram_protect, rc4030State),
+        VMSTATE_UINT32_ARRAY(rem_speed, rc4030State, 16),
+        VMSTATE_UINT32(imr_jazz, rc4030State),
+        VMSTATE_UINT32(isr_jazz, rc4030State),
+        VMSTATE_UINT32(itr, rc4030State),
+        VMSTATE_END_OF_LIST()
+    }
+};
 
 static void rc4030_do_dma(void *opaque, int n, uint8_t *buf, int len, int is_write)
 {
@@ -753,8 +727,6 @@ static void rc4030_initfn(Object *obj)
     sysbus_init_irq(sysbus, &s->timer_irq);
     sysbus_init_irq(sysbus, &s->jazz_bus_irq);
 
-    register_savevm(NULL, "rc4030", 0, 2, rc4030_save, rc4030_load, s);
-
     sysbus_init_mmio(sysbus, &s->iomem_chipset);
     sysbus_init_mmio(sysbus, &s->iomem_jazzio);
 }
@@ -813,6 +785,7 @@ static void rc4030_class_init(ObjectClass *klass, void *class_data)
     dc->realize = rc4030_realize;
     dc->unrealize = rc4030_unrealize;
     dc->reset = rc4030_reset;
+    dc->vmsd = &vmstate_rc4030;
 }
 
 static const TypeInfo rc4030_info = {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH v2 3/5] vmstateify ssi-sd
  2016-08-24 10:40 [Qemu-devel] [PATCH v2 0/5] vmstateify a bunch of rare devices [for 2.8] Dr. David Alan Gilbert (git)
  2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 1/5] vmstateify ssd0323 display Dr. David Alan Gilbert (git)
  2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 2/5] vmstateify rc4030 Dr. David Alan Gilbert (git)
@ 2016-08-24 10:40 ` Dr. David Alan Gilbert (git)
  2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 4/5] vmstateify tsc2005 Dr. David Alan Gilbert (git)
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2016-08-24 10:40 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, hpoussin; +Cc: amit.shah, quintela

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Changed a few types to fixed sized types in the ssi_sd_state
Now saving/loading a byte for the cmdarg/response bytes that were
  previously saved as uint32
Bumped version number to deal with those changes.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/sd/ssi-sd.c | 70 +++++++++++++++++++++++-----------------------------------
 1 file changed, 28 insertions(+), 42 deletions(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 3ff0886..24001dc 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -31,7 +31,7 @@ do { fprintf(stderr, "ssi_sd: error: " fmt , ## __VA_ARGS__);} while (0)
 #endif
 
 typedef enum {
-    SSI_SD_CMD,
+    SSI_SD_CMD = 0,
     SSI_SD_CMDARG,
     SSI_SD_RESPONSE,
     SSI_SD_DATA_START,
@@ -40,13 +40,13 @@ typedef enum {
 
 typedef struct {
     SSISlave ssidev;
-    ssi_sd_mode mode;
+    uint32_t mode;
     int cmd;
     uint8_t cmdarg[4];
     uint8_t response[5];
-    int arglen;
-    int response_pos;
-    int stopping;
+    int32_t arglen;
+    int32_t response_pos;
+    int32_t stopping;
     SDState *sd;
 } ssi_sd_state;
 
@@ -198,61 +198,46 @@ static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val)
     return 0xff;
 }
 
-static void ssi_sd_save(QEMUFile *f, void *opaque)
+static int ssi_sd_post_load(void *opaque, int version_id)
 {
-    SSISlave *ss = SSI_SLAVE(opaque);
     ssi_sd_state *s = (ssi_sd_state *)opaque;
-    int i;
 
-    qemu_put_be32(f, s->mode);
-    qemu_put_be32(f, s->cmd);
-    for (i = 0; i < 4; i++)
-        qemu_put_be32(f, s->cmdarg[i]);
-    for (i = 0; i < 5; i++)
-        qemu_put_be32(f, s->response[i]);
-    qemu_put_be32(f, s->arglen);
-    qemu_put_be32(f, s->response_pos);
-    qemu_put_be32(f, s->stopping);
-
-    qemu_put_be32(f, ss->cs);
-}
-
-static int ssi_sd_load(QEMUFile *f, void *opaque, int version_id)
-{
-    SSISlave *ss = SSI_SLAVE(opaque);
-    ssi_sd_state *s = (ssi_sd_state *)opaque;
-    int i;
-
-    if (version_id != 1)
+    if (s->mode > SSI_SD_DATA_READ) {
         return -EINVAL;
-
-    s->mode = qemu_get_be32(f);
-    s->cmd = qemu_get_be32(f);
-    for (i = 0; i < 4; i++)
-        s->cmdarg[i] = qemu_get_be32(f);
-    for (i = 0; i < 5; i++)
-        s->response[i] = qemu_get_be32(f);
-    s->arglen = qemu_get_be32(f);
+    }
     if (s->mode == SSI_SD_CMDARG &&
         (s->arglen < 0 || s->arglen >= ARRAY_SIZE(s->cmdarg))) {
         return -EINVAL;
     }
-    s->response_pos = qemu_get_be32(f);
-    s->stopping = qemu_get_be32(f);
     if (s->mode == SSI_SD_RESPONSE &&
         (s->response_pos < 0 || s->response_pos >= ARRAY_SIZE(s->response) ||
         (!s->stopping && s->arglen > ARRAY_SIZE(s->response)))) {
         return -EINVAL;
     }
 
-    ss->cs = qemu_get_be32(f);
-
     return 0;
 }
 
+static const VMStateDescription vmstate_ssi_sd = {
+    .name = "ssi_sd",
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .post_load = ssi_sd_post_load,
+    .fields = (VMStateField []) {
+        VMSTATE_UINT32(mode, ssi_sd_state),
+        VMSTATE_INT32(cmd, ssi_sd_state),
+        VMSTATE_UINT8_ARRAY(cmdarg, ssi_sd_state, 4),
+        VMSTATE_UINT8_ARRAY(response, ssi_sd_state, 5),
+        VMSTATE_INT32(arglen, ssi_sd_state),
+        VMSTATE_INT32(response_pos, ssi_sd_state),
+        VMSTATE_INT32(stopping, ssi_sd_state),
+        VMSTATE_SSI_SLAVE(ssidev, ssi_sd_state),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void ssi_sd_realize(SSISlave *d, Error **errp)
 {
-    DeviceState *dev = DEVICE(d);
     ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, d);
     DriveInfo *dinfo;
 
@@ -264,16 +249,17 @@ static void ssi_sd_realize(SSISlave *d, Error **errp)
         error_setg(errp, "Device initialization failed.");
         return;
     }
-    register_savevm(dev, "ssi_sd", -1, 1, ssi_sd_save, ssi_sd_load, s);
 }
 
 static void ssi_sd_class_init(ObjectClass *klass, void *data)
 {
+    DeviceClass *dc = DEVICE_CLASS(klass);
     SSISlaveClass *k = SSI_SLAVE_CLASS(klass);
 
     k->realize = ssi_sd_realize;
     k->transfer = ssi_sd_transfer;
     k->cs_polarity = SSI_CS_LOW;
+    dc->vmsd = &vmstate_ssi_sd;
 }
 
 static const TypeInfo ssi_sd_info = {
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH v2 4/5] vmstateify tsc2005
  2016-08-24 10:40 [Qemu-devel] [PATCH v2 0/5] vmstateify a bunch of rare devices [for 2.8] Dr. David Alan Gilbert (git)
                   ` (2 preceding siblings ...)
  2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 3/5] vmstateify ssi-sd Dr. David Alan Gilbert (git)
@ 2016-08-24 10:40 ` Dr. David Alan Gilbert (git)
  2016-09-22 15:41   ` Peter Maydell
  2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 5/5] vmstateify tsc210x Dr. David Alan Gilbert (git)
  2016-09-22 16:13 ` [Qemu-devel] [PATCH v2 0/5] vmstateify a bunch of rare devices [for 2.8] Peter Maydell
  5 siblings, 1 reply; 12+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2016-08-24 10:40 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, hpoussin; +Cc: amit.shah, quintela

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

I've converted the fields in it's main data structure
to fixed size types in ways that look sane, but I don't actually
know the details of this hardware.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/input/tsc2005.c | 154 ++++++++++++++++++++---------------------------------
 1 file changed, 57 insertions(+), 97 deletions(-)

diff --git a/hw/input/tsc2005.c b/hw/input/tsc2005.c
index 9b359aa..b66dc50 100644
--- a/hw/input/tsc2005.c
+++ b/hw/input/tsc2005.c
@@ -31,30 +31,31 @@ typedef struct {
     QEMUTimer *timer;
     uint16_t model;
 
-    int x, y;
-    int pressure;
+    int32_t x, y;
+    bool pressure;
 
-    int state, reg, irq, command;
+    uint8_t reg, state;
+    bool irq, command;
     uint16_t data, dav;
 
-    int busy;
-    int enabled;
-    int host_mode;
-    int function;
-    int nextfunction;
-    int precision;
-    int nextprecision;
-    int filter;
-    int pin_func;
-    int timing[2];
-    int noise;
-    int reset;
-    int pdst;
-    int pnd0;
+    bool busy;
+    bool enabled;
+    bool host_mode;
+    int8_t function;
+    int8_t nextfunction;
+    bool precision;
+    bool nextprecision;
+    uint16_t filter;
+    uint8_t pin_func;
+    int16_t timing[2];
+    uint8_t noise;
+    bool reset;
+    bool pdst;
+    bool pnd0;
     uint16_t temp_thr[2];
     uint16_t aux_thr[2];
 
-    int tr[8];
+    int32_t tr[8];
 } TSC2005State;
 
 enum {
@@ -434,86 +435,9 @@ static void tsc2005_touchscreen_event(void *opaque,
         tsc2005_pin_update(s);
 }
 
-static void tsc2005_save(QEMUFile *f, void *opaque)
+static int tsc2005_post_load(void *opaque, int version_id)
 {
     TSC2005State *s = (TSC2005State *) opaque;
-    int i;
-
-    qemu_put_be16(f, s->x);
-    qemu_put_be16(f, s->y);
-    qemu_put_byte(f, s->pressure);
-
-    qemu_put_byte(f, s->state);
-    qemu_put_byte(f, s->reg);
-    qemu_put_byte(f, s->command);
-
-    qemu_put_byte(f, s->irq);
-    qemu_put_be16s(f, &s->dav);
-    qemu_put_be16s(f, &s->data);
-
-    timer_put(f, s->timer);
-    qemu_put_byte(f, s->enabled);
-    qemu_put_byte(f, s->host_mode);
-    qemu_put_byte(f, s->function);
-    qemu_put_byte(f, s->nextfunction);
-    qemu_put_byte(f, s->precision);
-    qemu_put_byte(f, s->nextprecision);
-    qemu_put_be16(f, s->filter);
-    qemu_put_byte(f, s->pin_func);
-    qemu_put_be16(f, s->timing[0]);
-    qemu_put_be16(f, s->timing[1]);
-    qemu_put_be16s(f, &s->temp_thr[0]);
-    qemu_put_be16s(f, &s->temp_thr[1]);
-    qemu_put_be16s(f, &s->aux_thr[0]);
-    qemu_put_be16s(f, &s->aux_thr[1]);
-    qemu_put_be32(f, s->noise);
-    qemu_put_byte(f, s->reset);
-    qemu_put_byte(f, s->pdst);
-    qemu_put_byte(f, s->pnd0);
-
-    for (i = 0; i < 8; i ++)
-        qemu_put_be32(f, s->tr[i]);
-}
-
-static int tsc2005_load(QEMUFile *f, void *opaque, int version_id)
-{
-    TSC2005State *s = (TSC2005State *) opaque;
-    int i;
-
-    s->x = qemu_get_be16(f);
-    s->y = qemu_get_be16(f);
-    s->pressure = qemu_get_byte(f);
-
-    s->state = qemu_get_byte(f);
-    s->reg = qemu_get_byte(f);
-    s->command = qemu_get_byte(f);
-
-    s->irq = qemu_get_byte(f);
-    qemu_get_be16s(f, &s->dav);
-    qemu_get_be16s(f, &s->data);
-
-    timer_get(f, s->timer);
-    s->enabled = qemu_get_byte(f);
-    s->host_mode = qemu_get_byte(f);
-    s->function = qemu_get_byte(f);
-    s->nextfunction = qemu_get_byte(f);
-    s->precision = qemu_get_byte(f);
-    s->nextprecision = qemu_get_byte(f);
-    s->filter = qemu_get_be16(f);
-    s->pin_func = qemu_get_byte(f);
-    s->timing[0] = qemu_get_be16(f);
-    s->timing[1] = qemu_get_be16(f);
-    qemu_get_be16s(f, &s->temp_thr[0]);
-    qemu_get_be16s(f, &s->temp_thr[1]);
-    qemu_get_be16s(f, &s->aux_thr[0]);
-    qemu_get_be16s(f, &s->aux_thr[1]);
-    s->noise = qemu_get_be32(f);
-    s->reset = qemu_get_byte(f);
-    s->pdst = qemu_get_byte(f);
-    s->pnd0 = qemu_get_byte(f);
-
-    for (i = 0; i < 8; i ++)
-        s->tr[i] = qemu_get_be32(f);
 
     s->busy = timer_pending(s->timer);
     tsc2005_pin_update(s);
@@ -521,6 +445,42 @@ static int tsc2005_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
+static const VMStateDescription vmstate_tsc2005 = {
+    .name = "tsc2005",
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .post_load = tsc2005_post_load,
+    .fields      = (VMStateField []) {
+        VMSTATE_BOOL(pressure, TSC2005State),
+        VMSTATE_BOOL(irq, TSC2005State),
+        VMSTATE_BOOL(command, TSC2005State),
+        VMSTATE_BOOL(enabled, TSC2005State),
+        VMSTATE_BOOL(host_mode, TSC2005State),
+        VMSTATE_BOOL(reset, TSC2005State),
+        VMSTATE_BOOL(pdst, TSC2005State),
+        VMSTATE_BOOL(pnd0, TSC2005State),
+        VMSTATE_BOOL(precision, TSC2005State),
+        VMSTATE_BOOL(nextprecision, TSC2005State),
+        VMSTATE_UINT8(reg, TSC2005State),
+        VMSTATE_UINT8(state, TSC2005State),
+        VMSTATE_UINT16(data, TSC2005State),
+        VMSTATE_UINT16(dav, TSC2005State),
+        VMSTATE_UINT16(filter, TSC2005State),
+        VMSTATE_INT8(nextfunction, TSC2005State),
+        VMSTATE_INT8(function, TSC2005State),
+        VMSTATE_INT32(x, TSC2005State),
+        VMSTATE_INT32(y, TSC2005State),
+        VMSTATE_TIMER_PTR(timer, TSC2005State),
+        VMSTATE_UINT8(pin_func, TSC2005State),
+        VMSTATE_INT16_ARRAY(timing, TSC2005State, 2),
+        VMSTATE_UINT8(noise, TSC2005State),
+        VMSTATE_UINT16_ARRAY(temp_thr, TSC2005State, 2),
+        VMSTATE_UINT16_ARRAY(aux_thr, TSC2005State, 2),
+        VMSTATE_INT32_ARRAY(tr, TSC2005State, 8),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 void *tsc2005_init(qemu_irq pintdav)
 {
     TSC2005State *s;
@@ -550,7 +510,7 @@ void *tsc2005_init(qemu_irq pintdav)
                     "QEMU TSC2005-driven Touchscreen");
 
     qemu_register_reset((void *) tsc2005_reset, s);
-    register_savevm(NULL, "tsc2005", -1, 0, tsc2005_save, tsc2005_load, s);
+    vmstate_register(NULL, 0, &vmstate_tsc2005, s);
 
     return s;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Qemu-devel] [PATCH v2 5/5] vmstateify tsc210x
  2016-08-24 10:40 [Qemu-devel] [PATCH v2 0/5] vmstateify a bunch of rare devices [for 2.8] Dr. David Alan Gilbert (git)
                   ` (3 preceding siblings ...)
  2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 4/5] vmstateify tsc2005 Dr. David Alan Gilbert (git)
@ 2016-08-24 10:40 ` Dr. David Alan Gilbert (git)
  2016-09-22 16:12   ` Peter Maydell
  2016-09-22 16:13 ` [Qemu-devel] [PATCH v2 0/5] vmstateify a bunch of rare devices [for 2.8] Peter Maydell
  5 siblings, 1 reply; 12+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2016-08-24 10:40 UTC (permalink / raw)
  To: qemu-devel, peter.maydell, hpoussin; +Cc: amit.shah, quintela

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Note:
  I know nothing about this hardware.

  I'm now saving all 3 of the pll entries; only 2 were saved before.
  There are a couple of times that were previously stored as offsets
   from 'now' calculated before saving;  with vmstate it's easier
  to store the 'now' and fix it up on reload.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/input/tsc210x.c | 197 ++++++++++++++++++++++++-----------------------------
 1 file changed, 89 insertions(+), 108 deletions(-)

diff --git a/hw/input/tsc210x.c b/hw/input/tsc210x.c
index 93ca374..e7a7152 100644
--- a/hw/input/tsc210x.c
+++ b/hw/input/tsc210x.c
@@ -47,24 +47,25 @@ typedef struct {
     uint8_t out_fifo[16384];
     uint16_t model;
 
-    int x, y;
-    int pressure;
-
-    int state, page, offset, irq;
-    uint16_t command, dav;
-
-    int busy;
-    int enabled;
-    int host_mode;
-    int function;
-    int nextfunction;
-    int precision;
-    int nextprecision;
-    int filter;
-    int pin_func;
-    int ref;
-    int timing;
-    int noise;
+    int32_t x, y;
+    bool pressure;
+
+    uint8_t page, offset;
+    uint16_t dav;
+
+    bool state;
+    bool irq;
+    bool command;
+    bool busy;
+    bool enabled;
+    bool host_mode;
+    uint8_t function, nextfunction;
+    uint8_t precision, nextprecision;
+    uint8_t filter;
+    uint8_t pin_func;
+    uint8_t ref;
+    uint8_t timing;
+    uint8_t noise;
 
     uint16_t audio_ctrl1;
     uint16_t audio_ctrl2;
@@ -72,7 +73,7 @@ typedef struct {
     uint16_t pll[3];
     uint16_t volume;
     int64_t volume_change;
-    int softstep;
+    bool softstep;
     uint16_t dac_power;
     int64_t powerdown;
     uint16_t filter_data[0x14];
@@ -93,6 +94,7 @@ typedef struct {
         int mode;
         int intr;
     } kb;
+    int64_t now; /* Time at migration */
 } TSC210xState;
 
 static const int resolution[4] = { 12, 8, 10, 12 };
@@ -974,108 +976,34 @@ static void tsc210x_i2s_set_rate(TSC210xState *s, int in, int out)
     s->i2s_rx_rate = in;
 }
 
-static void tsc210x_save(QEMUFile *f, void *opaque)
+static void tsc210x_pre_save(void *opaque)
 {
     TSC210xState *s = (TSC210xState *) opaque;
-    int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-    int i;
-
-    qemu_put_be16(f, s->x);
-    qemu_put_be16(f, s->y);
-    qemu_put_byte(f, s->pressure);
-
-    qemu_put_byte(f, s->state);
-    qemu_put_byte(f, s->page);
-    qemu_put_byte(f, s->offset);
-    qemu_put_byte(f, s->command);
-
-    qemu_put_byte(f, s->irq);
-    qemu_put_be16s(f, &s->dav);
-
-    timer_put(f, s->timer);
-    qemu_put_byte(f, s->enabled);
-    qemu_put_byte(f, s->host_mode);
-    qemu_put_byte(f, s->function);
-    qemu_put_byte(f, s->nextfunction);
-    qemu_put_byte(f, s->precision);
-    qemu_put_byte(f, s->nextprecision);
-    qemu_put_byte(f, s->filter);
-    qemu_put_byte(f, s->pin_func);
-    qemu_put_byte(f, s->ref);
-    qemu_put_byte(f, s->timing);
-    qemu_put_be32(f, s->noise);
-
-    qemu_put_be16s(f, &s->audio_ctrl1);
-    qemu_put_be16s(f, &s->audio_ctrl2);
-    qemu_put_be16s(f, &s->audio_ctrl3);
-    qemu_put_be16s(f, &s->pll[0]);
-    qemu_put_be16s(f, &s->pll[1]);
-    qemu_put_be16s(f, &s->volume);
-    qemu_put_sbe64(f, (s->volume_change - now));
-    qemu_put_sbe64(f, (s->powerdown - now));
-    qemu_put_byte(f, s->softstep);
-    qemu_put_be16s(f, &s->dac_power);
-
-    for (i = 0; i < 0x14; i ++)
-        qemu_put_be16s(f, &s->filter_data[i]);
+    s->now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
 }
 
-static int tsc210x_load(QEMUFile *f, void *opaque, int version_id)
+static int tsc210x_post_load(void *opaque, int version_id)
 {
     TSC210xState *s = (TSC210xState *) opaque;
     int64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-    int i;
-
-    s->x = qemu_get_be16(f);
-    s->y = qemu_get_be16(f);
-    s->pressure = qemu_get_byte(f);
-
-    s->state = qemu_get_byte(f);
-    s->page = qemu_get_byte(f);
-    s->offset = qemu_get_byte(f);
-    s->command = qemu_get_byte(f);
 
-    s->irq = qemu_get_byte(f);
-    qemu_get_be16s(f, &s->dav);
-
-    timer_get(f, s->timer);
-    s->enabled = qemu_get_byte(f);
-    s->host_mode = qemu_get_byte(f);
-    s->function = qemu_get_byte(f);
-    if (s->function < 0 || s->function >= ARRAY_SIZE(mode_regs)) {
+    if (s->function >= ARRAY_SIZE(mode_regs)) {
         return -EINVAL;
     }
-    s->nextfunction = qemu_get_byte(f);
-    if (s->nextfunction < 0 || s->nextfunction >= ARRAY_SIZE(mode_regs)) {
+    if (s->nextfunction >= ARRAY_SIZE(mode_regs)) {
         return -EINVAL;
     }
-    s->precision = qemu_get_byte(f);
-    if (s->precision < 0 || s->precision >= ARRAY_SIZE(resolution)) {
+    if (s->precision >= ARRAY_SIZE(resolution)) {
         return -EINVAL;
     }
-    s->nextprecision = qemu_get_byte(f);
-    if (s->nextprecision < 0 || s->nextprecision >= ARRAY_SIZE(resolution)) {
+    if (s->nextprecision >= ARRAY_SIZE(resolution)) {
         return -EINVAL;
     }
-    s->filter = qemu_get_byte(f);
-    s->pin_func = qemu_get_byte(f);
-    s->ref = qemu_get_byte(f);
-    s->timing = qemu_get_byte(f);
-    s->noise = qemu_get_be32(f);
-
-    qemu_get_be16s(f, &s->audio_ctrl1);
-    qemu_get_be16s(f, &s->audio_ctrl2);
-    qemu_get_be16s(f, &s->audio_ctrl3);
-    qemu_get_be16s(f, &s->pll[0]);
-    qemu_get_be16s(f, &s->pll[1]);
-    qemu_get_be16s(f, &s->volume);
-    s->volume_change = qemu_get_sbe64(f) + now;
-    s->powerdown = qemu_get_sbe64(f) + now;
-    s->softstep = qemu_get_byte(f);
-    qemu_get_be16s(f, &s->dac_power);
-
-    for (i = 0; i < 0x14; i ++)
-        qemu_get_be16s(f, &s->filter_data[i]);
+
+    s->volume_change -= s->now;
+    s->volume_change += now;
+    s->powerdown -= s->now;
+    s->powerdown += now;
 
     s->busy = timer_pending(s->timer);
     qemu_set_irq(s->pint, !s->irq);
@@ -1084,6 +1012,60 @@ static int tsc210x_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }
 
+static VMStateField vmstatefields_tsc210x[] = {
+    VMSTATE_BOOL(enabled, TSC210xState),
+    VMSTATE_BOOL(host_mode, TSC210xState),
+    VMSTATE_BOOL(irq, TSC210xState),
+    VMSTATE_BOOL(command, TSC210xState),
+    VMSTATE_BOOL(pressure, TSC210xState),
+    VMSTATE_BOOL(softstep, TSC210xState),
+    VMSTATE_BOOL(state, TSC210xState),
+    VMSTATE_UINT16(dav, TSC210xState),
+    VMSTATE_INT32(x, TSC210xState),
+    VMSTATE_INT32(y, TSC210xState),
+    VMSTATE_UINT8(offset, TSC210xState),
+    VMSTATE_UINT8(page, TSC210xState),
+    VMSTATE_UINT8(filter, TSC210xState),
+    VMSTATE_UINT8(pin_func, TSC210xState),
+    VMSTATE_UINT8(ref, TSC210xState),
+    VMSTATE_UINT8(timing, TSC210xState),
+    VMSTATE_UINT8(noise, TSC210xState),
+    VMSTATE_UINT8(function, TSC210xState),
+    VMSTATE_UINT8(nextfunction, TSC210xState),
+    VMSTATE_UINT8(precision, TSC210xState),
+    VMSTATE_UINT8(nextprecision, TSC210xState),
+    VMSTATE_UINT16(audio_ctrl1, TSC210xState),
+    VMSTATE_UINT16(audio_ctrl2, TSC210xState),
+    VMSTATE_UINT16(audio_ctrl3, TSC210xState),
+    VMSTATE_UINT16_ARRAY(pll, TSC210xState, 3),
+    VMSTATE_UINT16(volume, TSC210xState),
+    VMSTATE_UINT16(dac_power, TSC210xState),
+    VMSTATE_INT64(volume_change, TSC210xState),
+    VMSTATE_INT64(powerdown, TSC210xState),
+    VMSTATE_INT64(now, TSC210xState),
+    VMSTATE_UINT16_ARRAY(filter_data, TSC210xState, 0x14),
+    VMSTATE_TIMER_PTR(timer, TSC210xState),
+    VMSTATE_END_OF_LIST()
+};
+
+static const VMStateDescription vmstate_tsc2102 = {
+    .name = "tsc2102",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .pre_save = tsc210x_pre_save,
+    .post_load = tsc210x_post_load,
+    .fields = vmstatefields_tsc210x,
+};
+
+static const VMStateDescription vmstate_tsc2301 = {
+    .name = "tsc2301",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .pre_save = tsc210x_pre_save,
+    .post_load = tsc210x_post_load,
+    .fields = vmstatefields_tsc210x,
+};
+
 uWireSlave *tsc2102_init(qemu_irq pint)
 {
     TSC210xState *s;
@@ -1125,8 +1107,7 @@ uWireSlave *tsc2102_init(qemu_irq pint)
     AUD_register_card(s->name, &s->card);
 
     qemu_register_reset((void *) tsc210x_reset, s);
-    register_savevm(NULL, s->name, -1, 0,
-                    tsc210x_save, tsc210x_load, s);
+    vmstate_register(NULL, 0, &vmstate_tsc2102, s);
 
     return &s->chip;
 }
@@ -1174,7 +1155,7 @@ uWireSlave *tsc2301_init(qemu_irq penirq, qemu_irq kbirq, qemu_irq dav)
     AUD_register_card(s->name, &s->card);
 
     qemu_register_reset((void *) tsc210x_reset, s);
-    register_savevm(NULL, s->name, -1, 0, tsc210x_save, tsc210x_load, s);
+    vmstate_register(NULL, 0, &vmstate_tsc2301, s);
 
     return &s->chip;
 }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/5] vmstateify tsc2005
  2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 4/5] vmstateify tsc2005 Dr. David Alan Gilbert (git)
@ 2016-09-22 15:41   ` Peter Maydell
  2016-09-22 15:53     ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 12+ messages in thread
From: Peter Maydell @ 2016-09-22 15:41 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git)
  Cc: QEMU Developers, Hervé Poussineau, Amit Shah, Juan Quintela

On 24 August 2016 at 11:40, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> I've converted the fields in it's main data structure
> to fixed size types in ways that look sane, but I don't actually
> know the details of this hardware.

This is the kind of thing that should go below the '---',
not in the commit message proper (at least not in this phrasing,
in my opinion).

The TSC2005 datasheet is http://www.ti.com/lit/ds/symlink/tsc2005.pdf .

> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/input/tsc2005.c | 154 ++++++++++++++++++++---------------------------------
>  1 file changed, 57 insertions(+), 97 deletions(-)
>
> diff --git a/hw/input/tsc2005.c b/hw/input/tsc2005.c
> index 9b359aa..b66dc50 100644
> --- a/hw/input/tsc2005.c
> +++ b/hw/input/tsc2005.c
> @@ -31,30 +31,31 @@ typedef struct {
>      QEMUTimer *timer;
>      uint16_t model;
>
> -    int x, y;
> -    int pressure;
> +    int32_t x, y;
> +    bool pressure;
>
> -    int state, reg, irq, command;
> +    uint8_t reg, state;
> +    bool irq, command;
>      uint16_t data, dav;
>
> -    int busy;
> -    int enabled;
> -    int host_mode;
> -    int function;
> -    int nextfunction;
> -    int precision;
> -    int nextprecision;
> -    int filter;
> -    int pin_func;
> -    int timing[2];
> -    int noise;
> -    int reset;
> -    int pdst;
> -    int pnd0;
> +    bool busy;
> +    bool enabled;

These changes mean the code is now doing bitwise-logic on
bool variables, for instance:
            s->busy &= s->enabled;

We also assign '0' and '1' to them rather than 'true' and 'false'.

> +    bool host_mode;
> +    int8_t function;
> +    int8_t nextfunction;
> +    bool precision;
> +    bool nextprecision;
> +    uint16_t filter;
> +    uint8_t pin_func;
> +    int16_t timing[2];

Why not uint16_t ?

> +    uint8_t noise;
> +    bool reset;
> +    bool pdst;
> +    bool pnd0;
>      uint16_t temp_thr[2];
>      uint16_t aux_thr[2];
>
> -    int tr[8];
> +    int32_t tr[8];
>  } TSC2005State;

Looks ok otherwise.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/5] vmstateify tsc2005
  2016-09-22 15:41   ` Peter Maydell
@ 2016-09-22 15:53     ` Dr. David Alan Gilbert
  2016-09-22 16:00       ` Peter Maydell
  0 siblings, 1 reply; 12+ messages in thread
From: Dr. David Alan Gilbert @ 2016-09-22 15:53 UTC (permalink / raw)
  To: Peter Maydell
  Cc: QEMU Developers, Hervé Poussineau, Amit Shah, Juan Quintela

* Peter Maydell (peter.maydell@linaro.org) wrote:
> On 24 August 2016 at 11:40, Dr. David Alan Gilbert (git)
> <dgilbert@redhat.com> wrote:
> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >
> > I've converted the fields in it's main data structure
> > to fixed size types in ways that look sane, but I don't actually
> > know the details of this hardware.
> 
> This is the kind of thing that should go below the '---',
> not in the commit message proper (at least not in this phrasing,
> in my opinion).

I can fix that.

> The TSC2005 datasheet is http://www.ti.com/lit/ds/symlink/tsc2005.pdf .
> 
> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  hw/input/tsc2005.c | 154 ++++++++++++++++++++---------------------------------
> >  1 file changed, 57 insertions(+), 97 deletions(-)
> >
> > diff --git a/hw/input/tsc2005.c b/hw/input/tsc2005.c
> > index 9b359aa..b66dc50 100644
> > --- a/hw/input/tsc2005.c
> > +++ b/hw/input/tsc2005.c
> > @@ -31,30 +31,31 @@ typedef struct {
> >      QEMUTimer *timer;
> >      uint16_t model;
> >
> > -    int x, y;
> > -    int pressure;
> > +    int32_t x, y;
> > +    bool pressure;
> >
> > -    int state, reg, irq, command;
> > +    uint8_t reg, state;
> > +    bool irq, command;
> >      uint16_t data, dav;
> >
> > -    int busy;
> > -    int enabled;
> > -    int host_mode;
> > -    int function;
> > -    int nextfunction;
> > -    int precision;
> > -    int nextprecision;
> > -    int filter;
> > -    int pin_func;
> > -    int timing[2];
> > -    int noise;
> > -    int reset;
> > -    int pdst;
> > -    int pnd0;
> > +    bool busy;
> > +    bool enabled;
> 
> These changes mean the code is now doing bitwise-logic on
> bool variables, for instance:
>             s->busy &= s->enabled;

> We also assign '0' and '1' to them rather than 'true' and 'false'.

Yes, I went through and as far as I can tell they're always
used as booleans already.


> > +    bool host_mode;
> > +    int8_t function;
> > +    int8_t nextfunction;
> > +    bool precision;
> > +    bool nextprecision;
> > +    uint16_t filter;
> > +    uint8_t pin_func;
> > +    int16_t timing[2];
> 
> Why not uint16_t ?

Yep, I'll fix that.

> > +    uint8_t noise;
> > +    bool reset;
> > +    bool pdst;
> > +    bool pnd0;
> >      uint16_t temp_thr[2];
> >      uint16_t aux_thr[2];
> >
> > -    int tr[8];
> > +    int32_t tr[8];
> >  } TSC2005State;
> 
> Looks ok otherwise.
> 
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2 4/5] vmstateify tsc2005
  2016-09-22 15:53     ` Dr. David Alan Gilbert
@ 2016-09-22 16:00       ` Peter Maydell
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2016-09-22 16:00 UTC (permalink / raw)
  To: Dr. David Alan Gilbert
  Cc: QEMU Developers, Hervé Poussineau, Amit Shah, Juan Quintela

On 22 September 2016 at 16:53, Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
> * Peter Maydell (peter.maydell@linaro.org) wrote:
>> These changes mean the code is now doing bitwise-logic on
>> bool variables, for instance:
>>             s->busy &= s->enabled;
>
>> We also assign '0' and '1' to them rather than 'true' and 'false'.
>
> Yes, I went through and as far as I can tell they're always
> used as booleans already.

Yeah, but using bitwise ops on booleans is the kind of
thing that prompts compilers and static analysers and
linters to complain, so we should fix those expressions
if we're changing the type.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2 5/5] vmstateify tsc210x
  2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 5/5] vmstateify tsc210x Dr. David Alan Gilbert (git)
@ 2016-09-22 16:12   ` Peter Maydell
  0 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2016-09-22 16:12 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git)
  Cc: QEMU Developers, Hervé Poussineau, Amit Shah, Juan Quintela

On 24 August 2016 at 11:40, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Note:
>   I know nothing about this hardware.
>
>   I'm now saving all 3 of the pll entries; only 2 were saved before.
>   There are a couple of times that were previously stored as offsets
>    from 'now' calculated before saving;  with vmstate it's easier
>   to store the 'now' and fix it up on reload.
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  hw/input/tsc210x.c | 197 ++++++++++++++++++++++++-----------------------------
>  1 file changed, 89 insertions(+), 108 deletions(-)

This one looks ok, other than the same comments as for 4/5.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2 0/5] vmstateify a bunch of rare devices [for 2.8]
  2016-08-24 10:40 [Qemu-devel] [PATCH v2 0/5] vmstateify a bunch of rare devices [for 2.8] Dr. David Alan Gilbert (git)
                   ` (4 preceding siblings ...)
  2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 5/5] vmstateify tsc210x Dr. David Alan Gilbert (git)
@ 2016-09-22 16:13 ` Peter Maydell
  5 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2016-09-22 16:13 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git)
  Cc: QEMU Developers, Hervé Poussineau, Amit Shah, Juan Quintela

On 24 August 2016 at 11:40, Dr. David Alan Gilbert (git)
<dgilbert@redhat.com> wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> In an effort to nibble away at register_savevm users,
> this set removes 5 more and converts them to vmstate.
>
> The ssd0323 seems to test OK with Peter's Stellaris test.
> The rc4030 has a tested-by from Hervé.
> The other 3 I'm less sure of; the ssi-sd is similar to the ssd0323;
> the two tsc2xxx devices I've not got a test for (or at least a test
> that works prior to the changes).
>
> (This is v2 since I previously posted the rc4030 and ssd0323 back in July;
> but it seems to make sense to roll this lot together)

rc4030 is mips so should go through the mips tree (also the
commit message IMHO needs cleanup). I've taken patches 1 and 3
into target-arm.next.

thanks
-- PMM

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Qemu-devel] [PATCH v2 2/5] vmstateify rc4030
  2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 2/5] vmstateify rc4030 Dr. David Alan Gilbert (git)
@ 2016-09-26 12:05   ` Dr. David Alan Gilbert
  0 siblings, 0 replies; 12+ messages in thread
From: Dr. David Alan Gilbert @ 2016-09-26 12:05 UTC (permalink / raw)
  To: qemu-devel, aurelien, yongbok.kim; +Cc: amit.shah, quintela

Hi Aurelien, Yongbok,
  Can you pick up this one MIPS patch from my vmstate series which is for
MIPS; Peter said he'd prefer it to go through your MIPS trees.

Thanks,

Dave

* Dr. David Alan Gilbert (git) (dgilbert@redhat.com) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> rc4030 seems to be part of a MIPS chipset; this converts it to
> VMState.
> 
>   Note:
>     a) It builds but I've not found a way to boot a MIPS Jazz image to
> test it.
>     b) It was saving 0..<15 on the 16 entry rem_speed array; I've not
> got a clue what that array is but I'm now saving the whole 16 entries
> rather than 15.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>
> Tested-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>  hw/dma/rc4030.c | 81 +++++++++++++++++++--------------------------------------
>  1 file changed, 27 insertions(+), 54 deletions(-)
> 
> diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
> index 2f2576f..17c8518 100644
> --- a/hw/dma/rc4030.c
> +++ b/hw/dma/rc4030.c
> @@ -616,34 +616,9 @@ static void rc4030_reset(DeviceState *dev)
>      qemu_irq_lower(s->jazz_bus_irq);
>  }
>  
> -static int rc4030_load(QEMUFile *f, void *opaque, int version_id)
> +static int rc4030_post_load(void *opaque, int version_id)
>  {
>      rc4030State* s = opaque;
> -    int i, j;
> -
> -    if (version_id != 2)
> -        return -EINVAL;
> -
> -    s->config = qemu_get_be32(f);
> -    s->invalid_address_register = qemu_get_be32(f);
> -    for (i = 0; i < 8; i++)
> -        for (j = 0; j < 4; j++)
> -            s->dma_regs[i][j] = qemu_get_be32(f);
> -    s->dma_tl_base = qemu_get_be32(f);
> -    s->dma_tl_limit = qemu_get_be32(f);
> -    s->cache_maint = qemu_get_be32(f);
> -    s->remote_failed_address = qemu_get_be32(f);
> -    s->memory_failed_address = qemu_get_be32(f);
> -    s->cache_ptag = qemu_get_be32(f);
> -    s->cache_ltag = qemu_get_be32(f);
> -    s->cache_bmask = qemu_get_be32(f);
> -    s->memory_refresh_rate = qemu_get_be32(f);
> -    s->nvram_protect = qemu_get_be32(f);
> -    for (i = 0; i < 15; i++)
> -        s->rem_speed[i] = qemu_get_be32(f);
> -    s->imr_jazz = qemu_get_be32(f);
> -    s->isr_jazz = qemu_get_be32(f);
> -    s->itr = qemu_get_be32(f);
>  
>      set_next_tick(s);
>      update_jazz_irq(s);
> @@ -651,32 +626,31 @@ static int rc4030_load(QEMUFile *f, void *opaque, int version_id)
>      return 0;
>  }
>  
> -static void rc4030_save(QEMUFile *f, void *opaque)
> -{
> -    rc4030State* s = opaque;
> -    int i, j;
> -
> -    qemu_put_be32(f, s->config);
> -    qemu_put_be32(f, s->invalid_address_register);
> -    for (i = 0; i < 8; i++)
> -        for (j = 0; j < 4; j++)
> -            qemu_put_be32(f, s->dma_regs[i][j]);
> -    qemu_put_be32(f, s->dma_tl_base);
> -    qemu_put_be32(f, s->dma_tl_limit);
> -    qemu_put_be32(f, s->cache_maint);
> -    qemu_put_be32(f, s->remote_failed_address);
> -    qemu_put_be32(f, s->memory_failed_address);
> -    qemu_put_be32(f, s->cache_ptag);
> -    qemu_put_be32(f, s->cache_ltag);
> -    qemu_put_be32(f, s->cache_bmask);
> -    qemu_put_be32(f, s->memory_refresh_rate);
> -    qemu_put_be32(f, s->nvram_protect);
> -    for (i = 0; i < 15; i++)
> -        qemu_put_be32(f, s->rem_speed[i]);
> -    qemu_put_be32(f, s->imr_jazz);
> -    qemu_put_be32(f, s->isr_jazz);
> -    qemu_put_be32(f, s->itr);
> -}
> +static const VMStateDescription vmstate_rc4030 = {
> +    .name = "rc4030",
> +    .version_id = 3,
> +    .post_load = rc4030_post_load,
> +    .fields = (VMStateField []) {
> +        VMSTATE_UINT32(config, rc4030State),
> +        VMSTATE_UINT32(invalid_address_register, rc4030State),
> +        VMSTATE_UINT32_2DARRAY(dma_regs, rc4030State, 8, 4),
> +        VMSTATE_UINT32(dma_tl_base, rc4030State),
> +        VMSTATE_UINT32(dma_tl_limit, rc4030State),
> +        VMSTATE_UINT32(cache_maint, rc4030State),
> +        VMSTATE_UINT32(remote_failed_address, rc4030State),
> +        VMSTATE_UINT32(memory_failed_address, rc4030State),
> +        VMSTATE_UINT32(cache_ptag, rc4030State),
> +        VMSTATE_UINT32(cache_ltag, rc4030State),
> +        VMSTATE_UINT32(cache_bmask, rc4030State),
> +        VMSTATE_UINT32(memory_refresh_rate, rc4030State),
> +        VMSTATE_UINT32(nvram_protect, rc4030State),
> +        VMSTATE_UINT32_ARRAY(rem_speed, rc4030State, 16),
> +        VMSTATE_UINT32(imr_jazz, rc4030State),
> +        VMSTATE_UINT32(isr_jazz, rc4030State),
> +        VMSTATE_UINT32(itr, rc4030State),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
>  
>  static void rc4030_do_dma(void *opaque, int n, uint8_t *buf, int len, int is_write)
>  {
> @@ -753,8 +727,6 @@ static void rc4030_initfn(Object *obj)
>      sysbus_init_irq(sysbus, &s->timer_irq);
>      sysbus_init_irq(sysbus, &s->jazz_bus_irq);
>  
> -    register_savevm(NULL, "rc4030", 0, 2, rc4030_save, rc4030_load, s);
> -
>      sysbus_init_mmio(sysbus, &s->iomem_chipset);
>      sysbus_init_mmio(sysbus, &s->iomem_jazzio);
>  }
> @@ -813,6 +785,7 @@ static void rc4030_class_init(ObjectClass *klass, void *class_data)
>      dc->realize = rc4030_realize;
>      dc->unrealize = rc4030_unrealize;
>      dc->reset = rc4030_reset;
> +    dc->vmsd = &vmstate_rc4030;
>  }
>  
>  static const TypeInfo rc4030_info = {
> -- 
> 2.7.4
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-09-26 12:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-08-24 10:40 [Qemu-devel] [PATCH v2 0/5] vmstateify a bunch of rare devices [for 2.8] Dr. David Alan Gilbert (git)
2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 1/5] vmstateify ssd0323 display Dr. David Alan Gilbert (git)
2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 2/5] vmstateify rc4030 Dr. David Alan Gilbert (git)
2016-09-26 12:05   ` Dr. David Alan Gilbert
2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 3/5] vmstateify ssi-sd Dr. David Alan Gilbert (git)
2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 4/5] vmstateify tsc2005 Dr. David Alan Gilbert (git)
2016-09-22 15:41   ` Peter Maydell
2016-09-22 15:53     ` Dr. David Alan Gilbert
2016-09-22 16:00       ` Peter Maydell
2016-08-24 10:40 ` [Qemu-devel] [PATCH v2 5/5] vmstateify tsc210x Dr. David Alan Gilbert (git)
2016-09-22 16:12   ` Peter Maydell
2016-09-22 16:13 ` [Qemu-devel] [PATCH v2 0/5] vmstateify a bunch of rare devices [for 2.8] Peter Maydell

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.