[Qemu-devel] [PATCH V2 00/10] SD save/load support and SD qomification

[Qemu-devel] [PATCH V2 00/10] SD save/load support and SD qomification

[Igor Mitsyanko]

v1->v2
PATCH1: use bitmap.h heder for bit operations.
PATCH6: use wrappers to call SDClass methods
Added 4 new patches:
PATCH7 and PATCH8 drope passing arguments to SDClass::init functions and replace them with
SD card object properties. SDClass::init is trivialized to realize().
PATCH10 adds "image" property to SD card objects. This property can be used for
hot-insertion/hot-cardchange in virtual SD card slot.

PATCH1 converts wp_groups member of SDState to bitfield significantly reducing
memory consumption.
PATCH2-4 convert binary variables to bool type.
PATCH5 adds save/load support for SDState, intermediate variable introduced in SDState
to hold size of wp_groups array.
PATCH6 converts SD state to QOM object. QOMified implementation doesn't have any
advantages over current one but it gives us enormous possibilities for further
improvements.

Igor Mitsyanko (10):
  hw/sd.c: convert wp_groups in SDState to bitfield
  hw/sd.c: convert binary variables to bool
  hw/sd.c: make sd_dataready() return bool
  hw/sd.c: make sd_wp_addr() return bool
  hw/sd.c: add SD card save/load support
  hw/sd.c: convert to QOM object
  SD card: introduce "if-idx" property for SD card objects
  SD card: introduce "spi_mode" property for SD card objects
  SD card: make SD card a child of host controller
  hw/sd.c: introduce SD card "image" property and allow SD hot-insert

 hw/milkymist-memcard.c |   31 ++++--
 hw/omap.h              |    4 +-
 hw/omap1.c             |    2 +-
 hw/omap2.c             |    2 +-
 hw/omap_mmc.c          |   42 +++++---
 hw/pl181.c             |   21 +++-
 hw/pxa.h               |    2 +-
 hw/pxa2xx.c            |    5 +-
 hw/pxa2xx_mmci.c       |   29 +++--
 hw/sd.c                |  301 ++++++++++++++++++++++++++++++++++++++----------
 hw/sd.h                |   38 +++++--
 hw/ssi-sd.c            |   18 ++-
 12 files changed, 374 insertions(+), 121 deletions(-)

-- 
1.7.4.1

[Qemu-devel] [PATCH V2 01/10] hw/sd.c: convert wp_groups in SDState to bitfield

[Igor Mitsyanko]

Representing each group write protection flag with only one bit instead of int
variable significantly reduces memory consumption.

Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
---
 hw/sd.c |   33 +++++++++++++++++++--------------
 1 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index 07eb263..f405b8d 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -32,6 +32,7 @@
 #include "hw.h"
 #include "block.h"
 #include "sd.h"
+#include "bitmap.h"
 
 //#define DEBUG_SD 1
 
@@ -81,7 +82,7 @@ struct SDState {
     uint8_t sd_status[64];
     uint32_t vhs;
     int wp_switch;
-    int *wp_groups;
+    unsigned long *wp_groups;
     uint64_t size;
     int blk_len;
     uint32_t erase_start;
@@ -415,7 +416,7 @@ static void sd_reset(SDState *sd, BlockDriverState *bdrv)
     if (sd->wp_groups)
         g_free(sd->wp_groups);
     sd->wp_switch = bdrv ? bdrv_is_read_only(bdrv) : 0;
-    sd->wp_groups = (int *) g_malloc0(sizeof(int) * sect);
+    sd->wp_groups = bitmap_new(sect);
     memset(sd->function_group, 0, sizeof(int) * 6);
     sd->erase_start = 0;
     sd->erase_end = 0;
@@ -484,9 +485,11 @@ static void sd_erase(SDState *sd)
     sd->erase_end = 0;
     sd->csd[14] |= 0x40;
 
-    for (i = start; i <= end; i ++)
-        if (sd->wp_groups[i])
+    for (i = start; i <= end; i++) {
+        if (test_bit(i, sd->wp_groups)) {
             sd->card_status |= WP_ERASE_SKIP;
+        }
+    }
 }
 
 static uint32_t sd_wpbits(SDState *sd, uint64_t addr)
@@ -496,9 +499,11 @@ static uint32_t sd_wpbits(SDState *sd, uint64_t addr)
 
     wpnum = addr >> (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT);
 
-    for (i = 0; i < 32; i ++, wpnum ++, addr += WPGROUP_SIZE)
-        if (addr < sd->size && sd->wp_groups[wpnum])
+    for (i = 0; i < 32; i++, wpnum++, addr += WPGROUP_SIZE) {
+        if (addr < sd->size && test_bit(wpnum, sd->wp_groups)) {
             ret |= (1 << i);
+        }
+    }
 
     return ret;
 }
@@ -536,8 +541,8 @@ static void sd_function_switch(SDState *sd, uint32_t arg)
 
 static inline int sd_wp_addr(SDState *sd, uint32_t addr)
 {
-    return sd->wp_groups[addr >>
-            (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT)];
+    return test_bit(addr >> (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT),
+            sd->wp_groups);
 }
 
 static void sd_lock_command(SDState *sd)
@@ -560,8 +565,8 @@ static void sd_lock_command(SDState *sd)
             sd->card_status |= LOCK_UNLOCK_FAILED;
             return;
         }
-        memset(sd->wp_groups, 0, sizeof(int) * (sd->size >>
-                        (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT)));
+        bitmap_zero(sd->wp_groups, BITS_TO_LONGS((sd->size >> (HWBLOCK_SHIFT +
+                SECTOR_SHIFT + WPGROUP_SHIFT)) + 1));
         sd->csd[14] &= ~0x10;
         sd->card_status &= ~CARD_IS_LOCKED;
         sd->pwd_len = 0;
@@ -1007,8 +1012,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             }
 
             sd->state = sd_programming_state;
-            sd->wp_groups[addr >> (HWBLOCK_SHIFT +
-                            SECTOR_SHIFT + WPGROUP_SHIFT)] = 1;
+            set_bit(addr >> (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT),
+                    sd->wp_groups);
             /* Bzzzzzzztt .... Operation complete.  */
             sd->state = sd_transfer_state;
             return sd_r1b;
@@ -1027,8 +1032,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
             }
 
             sd->state = sd_programming_state;
-            sd->wp_groups[addr >> (HWBLOCK_SHIFT +
-                            SECTOR_SHIFT + WPGROUP_SHIFT)] = 0;
+            clear_bit(addr >> (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT),
+                    sd->wp_groups);
             /* Bzzzzzzztt .... Operation complete.  */
             sd->state = sd_transfer_state;
             return sd_r1b;
-- 
1.7.4.1

[Qemu-devel] [PATCH V2 03/10] hw/sd.c: make sd_dataready() return bool

[Igor Mitsyanko]

For the sake of code clarity

Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sd.c |    2 +-
 hw/sd.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index eae1c20..e1c998c 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -1704,7 +1704,7 @@ uint8_t sd_read_data(SDState *sd)
     return ret;
 }
 
-int sd_data_ready(SDState *sd)
+bool sd_data_ready(SDState *sd)
 {
     return sd->state == sd_sendingdata_state;
 }
diff --git a/hw/sd.h b/hw/sd.h
index d25342f..4eb9679 100644
--- a/hw/sd.h
+++ b/hw/sd.h
@@ -73,7 +73,7 @@ int sd_do_command(SDState *sd, SDRequest *req,
 void sd_write_data(SDState *sd, uint8_t value);
 uint8_t sd_read_data(SDState *sd);
 void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert);
-int sd_data_ready(SDState *sd);
+bool sd_data_ready(SDState *sd);
 void sd_enable(SDState *sd, bool enable);
 
 #endif	/* __hw_sd_h */
-- 
1.7.4.1

[Qemu-devel] [PATCH V2 05/10] hw/sd.c: add SD card save/load support

[Igor Mitsyanko]

This patch updates SD card emulation to support save/load of card's state.

Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
---
 hw/sd.c |   89 +++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 files changed, 64 insertions(+), 25 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index 7160e8c..6f6be88 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -55,24 +55,28 @@ typedef enum {
     sd_illegal = -2,
 } sd_rsp_type_t;
 
+enum {
+    sd_inactive,
+    sd_card_identification_mode,
+    sd_data_transfer_mode,
+};
+
+enum {
+    sd_inactive_state = -1,
+    sd_idle_state = 0,
+    sd_ready_state,
+    sd_identification_state,
+    sd_standby_state,
+    sd_transfer_state,
+    sd_sendingdata_state,
+    sd_receivingdata_state,
+    sd_programming_state,
+    sd_disconnect_state,
+};
+
 struct SDState {
-    enum {
-        sd_inactive,
-        sd_card_identification_mode,
-        sd_data_transfer_mode,
-    } mode;
-    enum {
-        sd_inactive_state = -1,
-        sd_idle_state = 0,
-        sd_ready_state,
-        sd_identification_state,
-        sd_standby_state,
-        sd_transfer_state,
-        sd_sendingdata_state,
-        sd_receivingdata_state,
-        sd_programming_state,
-        sd_disconnect_state,
-    } state;
+    uint32_t mode;
+    int32_t state;
     uint32_t ocr;
     uint8_t scr[8];
     uint8_t cid[16];
@@ -83,21 +87,22 @@ struct SDState {
     uint32_t vhs;
     bool wp_switch;
     unsigned long *wp_groups;
+    uint32_t wpgrps_size;
     uint64_t size;
-    int blk_len;
+    uint32_t blk_len;
     uint32_t erase_start;
     uint32_t erase_end;
     uint8_t pwd[16];
-    int pwd_len;
-    int function_group[6];
+    uint32_t pwd_len;
+    uint8_t function_group[6];
 
     bool spi;
-    int current_cmd;
+    uint8_t current_cmd;
     /* True if we will handle the next command as an ACMD. Note that this does
      * *not* track the APP_CMD status bit!
      */
     bool expecting_acmd;
-    int blk_written;
+    uint32_t blk_written;
     uint64_t data_start;
     uint32_t data_offset;
     uint8_t data[512];
@@ -416,8 +421,9 @@ static void sd_reset(SDState *sd, BlockDriverState *bdrv)
     if (sd->wp_groups)
         g_free(sd->wp_groups);
     sd->wp_switch = bdrv ? bdrv_is_read_only(bdrv) : false;
+    sd->wpgrps_size = BITS_TO_LONGS(sect);
     sd->wp_groups = bitmap_new(sect);
-    memset(sd->function_group, 0, sizeof(int) * 6);
+    memset(sd->function_group, 0, sizeof(sd->function_group));
     sd->erase_start = 0;
     sd->erase_end = 0;
     sd->size = size;
@@ -441,6 +447,39 @@ static const BlockDevOps sd_block_ops = {
     .change_media_cb = sd_cardchange,
 };
 
+static const VMStateDescription sd_vmstate = {
+    .name = "sd-card",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .fields      = (VMStateField[]) {
+        VMSTATE_UINT32(mode, SDState),
+        VMSTATE_INT32(state, SDState),
+        VMSTATE_UINT8_ARRAY(cid, SDState, 16),
+        VMSTATE_UINT8_ARRAY(csd, SDState, 16),
+        VMSTATE_UINT16(rca, SDState),
+        VMSTATE_UINT32(card_status, SDState),
+        VMSTATE_PARTIAL_BUFFER(sd_status, SDState, 1),
+        VMSTATE_UINT32(vhs, SDState),
+        VMSTATE_BUFFER_MULTIPLY(wp_groups, SDState, 1, NULL, 0, wpgrps_size,
+                sizeof(unsigned long)),
+        VMSTATE_UINT32(blk_len, SDState),
+        VMSTATE_UINT32(erase_start, SDState),
+        VMSTATE_UINT32(erase_end, SDState),
+        VMSTATE_UINT8_ARRAY(pwd, SDState, 16),
+        VMSTATE_UINT32(pwd_len, SDState),
+        VMSTATE_UINT8_ARRAY(function_group, SDState, 6),
+        VMSTATE_UINT8(current_cmd, SDState),
+        VMSTATE_BOOL(expecting_acmd, SDState),
+        VMSTATE_UINT32(blk_written, SDState),
+        VMSTATE_UINT64(data_start, SDState),
+        VMSTATE_UINT32(data_offset, SDState),
+        VMSTATE_UINT8_ARRAY(data, SDState, 512),
+        VMSTATE_BUFFER_UNSAFE(buf, SDState, 1, 512),
+        VMSTATE_BOOL(enable, SDState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 /* We do not model the chip select pin, so allow the board to select
    whether card should be in SSI or MMC/SD mode.  It is also up to the
    board to ensure that ssi transfers only occur when the chip select
@@ -458,6 +497,7 @@ SDState *sd_init(BlockDriverState *bs, bool is_spi)
         bdrv_attach_dev_nofail(sd->bdrv, sd);
         bdrv_set_dev_ops(sd->bdrv, &sd_block_ops, sd);
     }
+    vmstate_register(NULL, -1, &sd_vmstate, sd);
     return sd;
 }
 
@@ -565,8 +605,7 @@ static void sd_lock_command(SDState *sd)
             sd->card_status |= LOCK_UNLOCK_FAILED;
             return;
         }
-        bitmap_zero(sd->wp_groups, BITS_TO_LONGS((sd->size >> (HWBLOCK_SHIFT +
-                SECTOR_SHIFT + WPGROUP_SHIFT)) + 1));
+        bitmap_zero(sd->wp_groups, sd->wpgrps_size);
         sd->csd[14] &= ~0x10;
         sd->card_status &= ~CARD_IS_LOCKED;
         sd->pwd_len = 0;
-- 
1.7.4.1

[Qemu-devel] [PATCH V2 06/10] hw/sd.c: convert to QOM object

[Igor Mitsyanko]

A straightforward conversion of SD card implementation to a proper QEMU object.

Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
---
 hw/milkymist-memcard.c |   24 ++++++++++++++----------
 hw/omap_mmc.c          |   28 ++++++++++++++++------------
 hw/pl181.c             |   14 ++++++++------
 hw/pxa2xx_mmci.c       |   22 ++++++++++++++--------
 hw/sd.c                |   48 +++++++++++++++++++++++++++++++++++++-----------
 hw/sd.h                |   38 ++++++++++++++++++++++++++++++--------
 hw/ssi-sd.c            |   11 ++++++-----
 7 files changed, 125 insertions(+), 60 deletions(-)

diff --git a/hw/milkymist-memcard.c b/hw/milkymist-memcard.c
index 3515c3c..1d84d44 100644
--- a/hw/milkymist-memcard.c
+++ b/hw/milkymist-memcard.c
@@ -97,7 +97,7 @@ static void memcard_sd_command(MilkymistMemcardState *s)
     req.crc = s->command[5];
 
     s->response[0] = req.cmd;
-    s->response_len = sd_do_command(s->card, &req, s->response+1);
+    s->response_len = SD_DO_COMMAND(s->card, &req, s->response + 1);
     s->response_read_ptr = 0;
 
     if (s->response_len == 16) {
@@ -141,11 +141,12 @@ static uint64_t memcard_read(void *opaque, target_phys_addr_t addr,
         if (!s->enabled) {
             r = 0xffffffff;
         } else {
+            SDClass *sd_class = SD_GET_CLASS(s->card);
             r = 0;
-            r |= sd_read_data(s->card) << 24;
-            r |= sd_read_data(s->card) << 16;
-            r |= sd_read_data(s->card) << 8;
-            r |= sd_read_data(s->card);
+            r |= sd_class->read_data(s->card) << 24;
+            r |= sd_class->read_data(s->card) << 16;
+            r |= sd_class->read_data(s->card) << 8;
+            r |= sd_class->read_data(s->card);
         }
         break;
     case R_CLK2XDIV:
@@ -170,6 +171,7 @@ static void memcard_write(void *opaque, target_phys_addr_t addr, uint64_t value,
                           unsigned size)
 {
     MilkymistMemcardState *s = opaque;
+    SDClass *sd_class;
 
     trace_milkymist_memcard_memory_write(addr, value);
 
@@ -198,10 +200,11 @@ static void memcard_write(void *opaque, target_phys_addr_t addr, uint64_t value,
         if (!s->enabled) {
             break;
         }
-        sd_write_data(s->card, (value >> 24) & 0xff);
-        sd_write_data(s->card, (value >> 16) & 0xff);
-        sd_write_data(s->card, (value >> 8) & 0xff);
-        sd_write_data(s->card, value & 0xff);
+        sd_class = SD_GET_CLASS(s->card);
+        sd_class->write_data(s->card, (value >> 24) & 0xff);
+        sd_class->write_data(s->card, (value >> 16) & 0xff);
+        sd_class->write_data(s->card, (value >> 8) & 0xff);
+        sd_class->write_data(s->card, value & 0xff);
         break;
     case R_ENABLE:
         s->regs[addr] = value;
@@ -249,8 +252,9 @@ static int milkymist_memcard_init(SysBusDevice *dev)
     MilkymistMemcardState *s = FROM_SYSBUS(typeof(*s), dev);
     DriveInfo *dinfo;
 
+    s->card = SD_CARD(object_new(TYPE_SD_CARD));
     dinfo = drive_get_next(IF_SD);
-    s->card = sd_init(dinfo ? dinfo->bdrv : NULL, 0);
+    SD_INIT(s->card, dinfo ? dinfo->bdrv : NULL, false);
     s->enabled = dinfo ? bdrv_is_inserted(dinfo->bdrv) : 0;
 
     memory_region_init_io(&s->regs_region, &memcard_mmio_ops, s,
diff --git a/hw/omap_mmc.c b/hw/omap_mmc.c
index aec0285..15bc1ae 100644
--- a/hw/omap_mmc.c
+++ b/hw/omap_mmc.c
@@ -138,7 +138,7 @@ static void omap_mmc_command(struct omap_mmc_s *host, int cmd, int dir,
     request.arg = host->arg;
     request.crc = 0; /* FIXME */
 
-    rsplen = sd_do_command(host->card, &request, response);
+    rsplen = SD_DO_COMMAND(host->card, &request, response);
 
     /* TODO: validate CRCs */
     switch (resptype) {
@@ -219,6 +219,7 @@ static void omap_mmc_command(struct omap_mmc_s *host, int cmd, int dir,
 
 static void omap_mmc_transfer(struct omap_mmc_s *host)
 {
+    SDClass *sd_class = SD_GET_CLASS(host->card);
     uint8_t value;
 
     if (!host->transfer)
@@ -229,10 +230,10 @@ static void omap_mmc_transfer(struct omap_mmc_s *host)
             if (host->fifo_len > host->af_level)
                 break;
 
-            value = sd_read_data(host->card);
+            value = sd_class->read_data(host->card);
             host->fifo[(host->fifo_start + host->fifo_len) & 31] = value;
             if (-- host->blen_counter) {
-                value = sd_read_data(host->card);
+                value = sd_class->read_data(host->card);
                 host->fifo[(host->fifo_start + host->fifo_len) & 31] |=
                         value << 8;
                 host->blen_counter --;
@@ -244,10 +245,10 @@ static void omap_mmc_transfer(struct omap_mmc_s *host)
                 break;
 
             value = host->fifo[host->fifo_start] & 0xff;
-            sd_write_data(host->card, value);
+            sd_class->write_data(host->card, value);
             if (-- host->blen_counter) {
                 value = host->fifo[host->fifo_start] >> 8;
-                sd_write_data(host->card, value);
+                sd_class->write_data(host->card, value);
                 host->blen_counter --;
             }
 
@@ -592,7 +593,8 @@ struct omap_mmc_s *omap_mmc_init(target_phys_addr_t base,
     memory_region_add_subregion(sysmem, base, &s->iomem);
 
     /* Instantiate the storage */
-    s->card = sd_init(bd, 0);
+    s->card = SD_CARD(object_new(TYPE_SD_CARD));
+    SD_INIT(s->card, bd, false);
 
     return s;
 }
@@ -617,10 +619,11 @@ struct omap_mmc_s *omap2_mmc_init(struct omap_target_agent_s *ta,
     omap_l4_attach(ta, 0, &s->iomem);
 
     /* Instantiate the storage */
-    s->card = sd_init(bd, 0);
+    s->card = SD_CARD(object_new(TYPE_SD_CARD));
+    SD_INIT(s->card, bd, false);
 
     s->cdet = qemu_allocate_irqs(omap_mmc_cover_cb, s, 1)[0];
-    sd_set_cb(s->card, NULL, s->cdet);
+    SD_SET_CB(s->card, NULL, s->cdet);
 
     return s;
 }
@@ -628,14 +631,15 @@ struct omap_mmc_s *omap2_mmc_init(struct omap_target_agent_s *ta,
 void omap_mmc_handlers(struct omap_mmc_s *s, qemu_irq ro, qemu_irq cover)
 {
     if (s->cdet) {
-        sd_set_cb(s->card, ro, s->cdet);
+        SD_SET_CB(s->card, ro, s->cdet);
         s->coverswitch = cover;
         qemu_set_irq(cover, s->cdet_state);
-    } else
-        sd_set_cb(s->card, ro, cover);
+    } else {
+        SD_SET_CB(s->card, ro, cover);
+    }
 }
 
 void omap_mmc_enable(struct omap_mmc_s *s, int enable)
 {
-    sd_enable(s->card, enable);
+    SD_ENABLE(s->card, !!enable);
 }
diff --git a/hw/pl181.c b/hw/pl181.c
index 7d91fbb..b2b1ed4 100644
--- a/hw/pl181.c
+++ b/hw/pl181.c
@@ -171,7 +171,7 @@ static void pl181_send_command(pl181_state *s)
     request.cmd = s->cmd & PL181_CMD_INDEX;
     request.arg = s->cmdarg;
     DPRINTF("Command %d %08x\n", request.cmd, request.arg);
-    rlen = sd_do_command(s->card, &request, response);
+    rlen = SD_DO_COMMAND(s->card, &request, response);
     if (rlen < 0)
         goto error;
     if (s->cmd & PL181_CMD_RESPONSE) {
@@ -209,18 +209,19 @@ error:
 
 static void pl181_fifo_run(pl181_state *s)
 {
+    SDClass *sd_class = SD_GET_CLASS(s->card);
     uint32_t bits;
     uint32_t value = 0;
     int n;
     int is_read;
 
     is_read = (s->datactrl & PL181_DATA_DIRECTION) != 0;
-    if (s->datacnt != 0 && (!is_read || sd_data_ready(s->card))
+    if (s->datacnt != 0 && (!is_read || sd_class->data_ready(s->card))
             && !s->linux_hack) {
         if (is_read) {
             n = 0;
             while (s->datacnt && s->fifo_len < PL181_FIFO_LEN) {
-                value |= (uint32_t)sd_read_data(s->card) << (n * 8);
+                value |= (uint32_t)sd_class->read_data(s->card) << (n * 8);
                 s->datacnt--;
                 n++;
                 if (n == 4) {
@@ -241,7 +242,7 @@ static void pl181_fifo_run(pl181_state *s)
                 }
                 n--;
                 s->datacnt--;
-                sd_write_data(s->card, value & 0xff);
+                sd_class->write_data(s->card, value & 0xff);
                 value >>= 8;
             }
         }
@@ -469,7 +470,7 @@ static void pl181_reset(DeviceState *d)
     s->mask[1] = 0;
 
     /* We can assume our GPIO outputs have been wired up now */
-    sd_set_cb(s->card, s->cardstatus[0], s->cardstatus[1]);
+    SD_SET_CB(s->card, s->cardstatus[0], s->cardstatus[1]);
 }
 
 static int pl181_init(SysBusDevice *dev)
@@ -483,7 +484,8 @@ static int pl181_init(SysBusDevice *dev)
     sysbus_init_irq(dev, &s->irq[1]);
     qdev_init_gpio_out(&s->busdev.qdev, s->cardstatus, 2);
     dinfo = drive_get_next(IF_SD);
-    s->card = sd_init(dinfo ? dinfo->bdrv : NULL, 0);
+    s->card = SD_CARD(object_new(TYPE_SD_CARD));
+    SD_INIT(s->card, dinfo ? dinfo->bdrv : NULL, false);
     return 0;
 }
 
diff --git a/hw/pxa2xx_mmci.c b/hw/pxa2xx_mmci.c
index b505a4c..a7081b0 100644
--- a/hw/pxa2xx_mmci.c
+++ b/hw/pxa2xx_mmci.c
@@ -117,25 +117,30 @@ static void pxa2xx_mmci_int_update(PXA2xxMMCIState *s)
 
 static void pxa2xx_mmci_fifo_update(PXA2xxMMCIState *s)
 {
-    if (!s->active)
+    SDClass *sd_class = SD_GET_CLASS(s->card);
+
+    if (!s->active) {
         return;
+    }
 
     if (s->cmdat & CMDAT_WR_RD) {
         while (s->bytesleft && s->tx_len) {
-            sd_write_data(s->card, s->tx_fifo[s->tx_start ++]);
+            sd_class->write_data(s->card, s->tx_fifo[s->tx_start++]);
             s->tx_start &= 0x1f;
             s->tx_len --;
             s->bytesleft --;
         }
-        if (s->bytesleft)
+        if (s->bytesleft) {
             s->intreq |= INT_TXFIFO_REQ;
-    } else
+        }
+    } else {
         while (s->bytesleft && s->rx_len < 32) {
             s->rx_fifo[(s->rx_start + (s->rx_len ++)) & 0x1f] =
-                sd_read_data(s->card);
+                sd_class->read_data(s->card);
             s->bytesleft --;
             s->intreq |= INT_RXFIFO_REQ;
         }
+    }
 
     if (!s->bytesleft) {
         s->active = 0;
@@ -166,7 +171,7 @@ static void pxa2xx_mmci_wakequeues(PXA2xxMMCIState *s)
     request.arg = s->arg;
     request.crc = 0;	/* FIXME */
 
-    rsplen = sd_do_command(s->card, &request, response);
+    rsplen = SD_DO_COMMAND(s->card, &request, response);
     s->intreq |= INT_END_CMD;
 
     memset(s->resp_fifo, 0, sizeof(s->resp_fifo));
@@ -538,7 +543,8 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
     memory_region_add_subregion(sysmem, base, &s->iomem);
 
     /* Instantiate the actual storage */
-    s->card = sd_init(bd, 0);
+    s->card = SD_CARD(object_new(TYPE_SD_CARD));
+    SD_INIT(s->card, bd, false);
 
     register_savevm(NULL, "pxa2xx_mmci", 0, 0,
                     pxa2xx_mmci_save, pxa2xx_mmci_load, s);
@@ -549,5 +555,5 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
 void pxa2xx_mmci_handlers(PXA2xxMMCIState *s, qemu_irq readonly,
                 qemu_irq coverswitch)
 {
-    sd_set_cb(s->card, readonly, coverswitch);
+    SD_SET_CB(s->card, readonly, coverswitch);
 }
diff --git a/hw/sd.c b/hw/sd.c
index 6f6be88..58d94ed 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -75,6 +75,8 @@ enum {
 };
 
 struct SDState {
+    Object parent_obj;
+
     uint32_t mode;
     int32_t state;
     uint32_t ocr;
@@ -484,11 +486,8 @@ static const VMStateDescription sd_vmstate = {
    whether card should be in SSI or MMC/SD mode.  It is also up to the
    board to ensure that ssi transfers only occur when the chip select
    is asserted.  */
-SDState *sd_init(BlockDriverState *bs, bool is_spi)
+static void sd_init(SDState *sd, BlockDriverState *bs, bool is_spi)
 {
-    SDState *sd;
-
-    sd = (SDState *) g_malloc0(sizeof(SDState));
     sd->buf = qemu_blockalign(bs, 512);
     sd->spi = is_spi;
     sd->enable = true;
@@ -498,10 +497,9 @@ SDState *sd_init(BlockDriverState *bs, bool is_spi)
         bdrv_set_dev_ops(sd->bdrv, &sd_block_ops, sd);
     }
     vmstate_register(NULL, -1, &sd_vmstate, sd);
-    return sd;
 }
 
-void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
+static void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
 {
     sd->readonly_cb = readonly;
     sd->inserted_cb = insert;
@@ -1332,7 +1330,7 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest *req)
     return sd_cmd_class[req->cmd] == 0 || sd_cmd_class[req->cmd] == 7;
 }
 
-int sd_do_command(SDState *sd, SDRequest *req,
+static int sd_do_command(SDState *sd, SDRequest *req,
                   uint8_t *response) {
     int last_state;
     sd_rsp_type_t rtype;
@@ -1500,7 +1498,7 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
 #define APP_READ_BLOCK(a, len)	memset(sd->data, 0xec, len)
 #define APP_WRITE_BLOCK(a, len)
 
-void sd_write_data(SDState *sd, uint8_t value)
+static void sd_write_data(SDState *sd, uint8_t value)
 {
     int i;
 
@@ -1624,7 +1622,7 @@ void sd_write_data(SDState *sd, uint8_t value)
     }
 }
 
-uint8_t sd_read_data(SDState *sd)
+static uint8_t sd_read_data(SDState *sd)
 {
     /* TODO: Append CRCs */
     uint8_t ret;
@@ -1743,12 +1741,40 @@ uint8_t sd_read_data(SDState *sd)
     return ret;
 }
 
-bool sd_data_ready(SDState *sd)
+static bool sd_data_ready(SDState *sd)
 {
     return sd->state == sd_sendingdata_state;
 }
 
-void sd_enable(SDState *sd, bool enable)
+static void sd_enable(SDState *sd, bool enable)
 {
     sd->enable = enable;
 }
+
+static void sd_class_init(ObjectClass *class, void *data)
+{
+    SDClass *k = SD_CLASS(class);
+
+    k->init = sd_init;
+    k->set_cb = sd_set_cb;
+    k->do_command = sd_do_command;
+    k->data_ready = sd_data_ready;
+    k->read_data = sd_read_data;
+    k->write_data = sd_write_data;
+    k->enable = sd_enable;
+}
+
+static TypeInfo sd_type_info = {
+    .name = TYPE_SD_CARD,
+    .parent = TYPE_OBJECT,
+    .instance_size = sizeof(SDState),
+    .class_init = sd_class_init,
+    .class_size = sizeof(SDClass)
+};
+
+static void sd_register_type(void)
+{
+    type_register_static(&sd_type_info);
+}
+
+type_init(sd_register_type)
diff --git a/hw/sd.h b/hw/sd.h
index 4eb9679..d8f0195 100644
--- a/hw/sd.h
+++ b/hw/sd.h
@@ -29,6 +29,9 @@
 #ifndef __hw_sd_h
 #define __hw_sd_h		1
 
+#include "qemu-common.h"
+#include "qemu/object.h"
+
 #define OUT_OF_RANGE		(1 << 31)
 #define ADDRESS_ERROR		(1 << 30)
 #define BLOCK_LEN_ERROR		(1 << 29)
@@ -67,13 +70,32 @@ typedef struct {
 
 typedef struct SDState SDState;
 
-SDState *sd_init(BlockDriverState *bs, bool is_spi);
-int sd_do_command(SDState *sd, SDRequest *req,
-                  uint8_t *response);
-void sd_write_data(SDState *sd, uint8_t value);
-uint8_t sd_read_data(SDState *sd);
-void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert);
-bool sd_data_ready(SDState *sd);
-void sd_enable(SDState *sd, bool enable);
+typedef struct SDClass {
+    ObjectClass parent_class;
+
+    void (*init)(SDState *sd, BlockDriverState *bs, bool is_spi);
+    int (*do_command)(SDState *sd, SDRequest *req, uint8_t *response);
+    void (*write_data)(SDState *sd, uint8_t value);
+    uint8_t (*read_data)(SDState *sd);
+    void (*set_cb)(SDState *sd, qemu_irq readonly, qemu_irq insert);
+    bool (*data_ready)(SDState *sd);
+    void (*enable)(SDState *sd, bool enable);
+} SDClass;
+
+#define TYPE_SD_CARD            "sd-card"
+#define SD_CARD(obj)            \
+     OBJECT_CHECK(SDState, (obj), TYPE_SD_CARD)
+#define SD_CLASS(klass)         \
+     OBJECT_CLASS_CHECK(SDClass, (klass), TYPE_SD_CARD)
+#define SD_GET_CLASS(obj)       \
+     OBJECT_GET_CLASS(SDClass, (obj), TYPE_SD_CARD)
+
+#define SD_INIT(sd, bdrv, is_spi)   (SD_GET_CLASS(sd)->init(sd, bdrv, is_spi))
+#define SD_DO_COMMAND(sd, req, rsp) (SD_GET_CLASS(sd)->do_command(sd, req, rsp))
+#define SD_WRITE(sd, value)         (SD_GET_CLASS(sd)->write_data(sd, value))
+#define SD_READ(sd)                 (SD_GET_CLASS(sd)->read_data(sd))
+#define SD_SET_CB(sd, ro, ins)      (SD_GET_CLASS(sd)->set_cb(sd, ro, ins))
+#define SD_DATA_READY(sd)           (SD_GET_CLASS(sd)->data_ready(sd))
+#define SD_ENABLE(sd, enbl)         (SD_GET_CLASS(sd)->enable(sd, enbl))
 
 #endif	/* __hw_sd_h */
diff --git a/hw/ssi-sd.c b/hw/ssi-sd.c
index b519bdb..beecc0e 100644
--- a/hw/ssi-sd.c
+++ b/hw/ssi-sd.c
@@ -94,7 +94,7 @@ static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val)
             request.arg = (s->cmdarg[0] << 24) | (s->cmdarg[1] << 16)
                            | (s->cmdarg[2] << 8) | s->cmdarg[3];
             DPRINTF("CMD%d arg 0x%08x\n", s->cmd, request.arg);
-            s->arglen = sd_do_command(s->sd, &request, longresp);
+            s->arglen = SD_DO_COMMAND(s->sd, &request, longresp);
             if (s->arglen <= 0) {
                 s->arglen = 1;
                 s->response[0] = 4;
@@ -171,7 +171,7 @@ static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val)
             DPRINTF("Response 0x%02x\n", s->response[s->response_pos]);
             return s->response[s->response_pos++];
         }
-        if (sd_data_ready(s->sd)) {
+        if (SD_DATA_READY(s->sd)) {
             DPRINTF("Data read\n");
             s->mode = SSI_SD_DATA_START;
         } else {
@@ -184,8 +184,8 @@ static uint32_t ssi_sd_transfer(SSISlave *dev, uint32_t val)
         s->mode = SSI_SD_DATA_READ;
         return 0xfe;
     case SSI_SD_DATA_READ:
-        val = sd_read_data(s->sd);
-        if (!sd_data_ready(s->sd)) {
+        val = SD_READ(s->sd);
+        if (!SD_DATA_READY(s->sd)) {
             DPRINTF("Data read end\n");
             s->mode = SSI_SD_CMD;
         }
@@ -239,7 +239,8 @@ static int ssi_sd_init(SSISlave *dev)
 
     s->mode = SSI_SD_CMD;
     dinfo = drive_get_next(IF_SD);
-    s->sd = sd_init(dinfo ? dinfo->bdrv : NULL, 1);
+    s->sd = SD_CARD(object_new(TYPE_SD_CARD));
+    SD_INIT(s->sd, dinfo ? dinfo->bdrv : NULL, true);
     register_savevm(&dev->qdev, "ssi_sd", -1, 1, ssi_sd_save, ssi_sd_load, s);
     return 0;
 }
-- 
1.7.4.1

[Qemu-devel] [PATCH V2 07/10] SD card: introduce "if-idx" property for SD card objects

[Igor Mitsyanko]

Rather then pass on a BlockDriverState pointer to SD init function, we now should set
"if-idx" property of SD card object to a value corresponding to index of DriveInfo
we want to attach to SD card.
All users were converted to use this new approach to SD card initialization. Omap and PXA
mmc initialization functions are modified to accept DriveInfo * instead of BlockDriverState *,
so these function may have easy access to block device coresponding index.
Host controllers should set "if-idx" property before initializing freshly instantiated
SD card. During initialization, card will be linked with block device of the same interface
index.

Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
---
 hw/milkymist-memcard.c |    7 ++++++-
 hw/omap.h              |    4 ++--
 hw/omap1.c             |    2 +-
 hw/omap2.c             |    2 +-
 hw/omap_mmc.c          |   16 ++++++++++++----
 hw/pl181.c             |    7 ++++++-
 hw/pxa.h               |    2 +-
 hw/pxa2xx.c            |    5 ++---
 hw/pxa2xx_mmci.c       |    8 ++++++--
 hw/sd.c                |   47 ++++++++++++++++++++++++++++++++++++++++++++---
 hw/sd.h                |    4 ++--
 hw/ssi-sd.c            |    7 ++++++-
 12 files changed, 89 insertions(+), 22 deletions(-)

diff --git a/hw/milkymist-memcard.c b/hw/milkymist-memcard.c
index 1d84d44..2fff47f 100644
--- a/hw/milkymist-memcard.c
+++ b/hw/milkymist-memcard.c
@@ -251,10 +251,15 @@ static int milkymist_memcard_init(SysBusDevice *dev)
 {
     MilkymistMemcardState *s = FROM_SYSBUS(typeof(*s), dev);
     DriveInfo *dinfo;
+    Error *errp = NULL;
 
     s->card = SD_CARD(object_new(TYPE_SD_CARD));
     dinfo = drive_get_next(IF_SD);
-    SD_INIT(s->card, dinfo ? dinfo->bdrv : NULL, false);
+    if (dinfo) {
+        object_property_set_int(OBJECT(s->card), dinfo->unit, "if-idx", &errp);
+    }
+    assert_no_error(errp);
+    SD_INIT(s->card, false);
     s->enabled = dinfo ? bdrv_is_inserted(dinfo->bdrv) : 0;
 
     memory_region_init_io(&s->regs_region, &memcard_mmio_ops, s,
diff --git a/hw/omap.h b/hw/omap.h
index 6c3d004..969f9cd 100644
--- a/hw/omap.h
+++ b/hw/omap.h
@@ -754,10 +754,10 @@ void omap_rfbi_attach(struct omap_dss_s *s, int cs, struct rfbi_chip_s *chip);
 struct omap_mmc_s;
 struct omap_mmc_s *omap_mmc_init(target_phys_addr_t base,
                 MemoryRegion *sysmem,
-                BlockDriverState *bd,
+                DriveInfo *di,
                 qemu_irq irq, qemu_irq dma[], omap_clk clk);
 struct omap_mmc_s *omap2_mmc_init(struct omap_target_agent_s *ta,
-                BlockDriverState *bd, qemu_irq irq, qemu_irq dma[],
+                DriveInfo *di, qemu_irq irq, qemu_irq dma[],
                 omap_clk fclk, omap_clk iclk);
 void omap_mmc_reset(struct omap_mmc_s *s);
 void omap_mmc_handlers(struct omap_mmc_s *s, qemu_irq ro, qemu_irq cover);
diff --git a/hw/omap1.c b/hw/omap1.c
index 80d47f0..fb722bc 100644
--- a/hw/omap1.c
+++ b/hw/omap1.c
@@ -3964,7 +3964,7 @@ struct omap_mpu_state_s *omap310_mpu_init(MemoryRegion *system_memory,
         fprintf(stderr, "qemu: missing SecureDigital device\n");
         exit(1);
     }
-    s->mmc = omap_mmc_init(0xfffb7800, system_memory, dinfo->bdrv,
+    s->mmc = omap_mmc_init(0xfffb7800, system_memory, dinfo,
                            qdev_get_gpio_in(s->ih[1], OMAP_INT_OQN),
                            &s->drq[OMAP_DMA_MMC_TX],
                     omap_findclk(s, "mmc_ck"));
diff --git a/hw/omap2.c b/hw/omap2.c
index 42fce5e..770389b 100644
--- a/hw/omap2.c
+++ b/hw/omap2.c
@@ -2459,7 +2459,7 @@ struct omap_mpu_state_s *omap2420_mpu_init(MemoryRegion *sysmem,
         fprintf(stderr, "qemu: missing SecureDigital device\n");
         exit(1);
     }
-    s->mmc = omap2_mmc_init(omap_l4tao(s->l4, 9), dinfo->bdrv,
+    s->mmc = omap2_mmc_init(omap_l4tao(s->l4, 9), dinfo,
                     qdev_get_gpio_in(s->ih[0], OMAP_INT_24XX_MMC_IRQ),
                     &s->drq[OMAP24XX_DMA_MMC1_TX],
                     omap_findclk(s, "mmc_fclk"), omap_findclk(s, "mmc_iclk"));
diff --git a/hw/omap_mmc.c b/hw/omap_mmc.c
index 15bc1ae..3bf29c2 100644
--- a/hw/omap_mmc.c
+++ b/hw/omap_mmc.c
@@ -19,6 +19,8 @@
 #include "hw.h"
 #include "omap.h"
 #include "sd.h"
+#include "qerror.h"
+#include "blockdev.h"
 
 struct omap_mmc_s {
     qemu_irq irq;
@@ -575,9 +577,10 @@ static void omap_mmc_cover_cb(void *opaque, int line, int level)
 
 struct omap_mmc_s *omap_mmc_init(target_phys_addr_t base,
                 MemoryRegion *sysmem,
-                BlockDriverState *bd,
+                DriveInfo *di,
                 qemu_irq irq, qemu_irq dma[], omap_clk clk)
 {
+    Error *errp = NULL;
     struct omap_mmc_s *s = (struct omap_mmc_s *)
             g_malloc0(sizeof(struct omap_mmc_s));
 
@@ -594,15 +597,18 @@ struct omap_mmc_s *omap_mmc_init(target_phys_addr_t base,
 
     /* Instantiate the storage */
     s->card = SD_CARD(object_new(TYPE_SD_CARD));
-    SD_INIT(s->card, bd, false);
+    object_property_set_int(OBJECT(s->card), di->unit, "if-idx", &errp);
+    assert_no_error(errp);
+    SD_INIT(s->card, false);
 
     return s;
 }
 
 struct omap_mmc_s *omap2_mmc_init(struct omap_target_agent_s *ta,
-                BlockDriverState *bd, qemu_irq irq, qemu_irq dma[],
+                DriveInfo *di, qemu_irq irq, qemu_irq dma[],
                 omap_clk fclk, omap_clk iclk)
 {
+    Error *errp = NULL;
     struct omap_mmc_s *s = (struct omap_mmc_s *)
             g_malloc0(sizeof(struct omap_mmc_s));
 
@@ -620,7 +626,9 @@ struct omap_mmc_s *omap2_mmc_init(struct omap_target_agent_s *ta,
 
     /* Instantiate the storage */
     s->card = SD_CARD(object_new(TYPE_SD_CARD));
-    SD_INIT(s->card, bd, false);
+    object_property_set_int(OBJECT(s->card), di->unit, "if-idx", &errp);
+    assert_no_error(errp);
+    SD_INIT(s->card, false);
 
     s->cdet = qemu_allocate_irqs(omap_mmc_cover_cb, s, 1)[0];
     SD_SET_CB(s->card, NULL, s->cdet);
diff --git a/hw/pl181.c b/hw/pl181.c
index b2b1ed4..a0a8fe3 100644
--- a/hw/pl181.c
+++ b/hw/pl181.c
@@ -477,6 +477,7 @@ static int pl181_init(SysBusDevice *dev)
 {
     pl181_state *s = FROM_SYSBUS(pl181_state, dev);
     DriveInfo *dinfo;
+    Error *errp = NULL;
 
     memory_region_init_io(&s->iomem, &pl181_ops, s, "pl181", 0x1000);
     sysbus_init_mmio(dev, &s->iomem);
@@ -485,7 +486,11 @@ static int pl181_init(SysBusDevice *dev)
     qdev_init_gpio_out(&s->busdev.qdev, s->cardstatus, 2);
     dinfo = drive_get_next(IF_SD);
     s->card = SD_CARD(object_new(TYPE_SD_CARD));
-    SD_INIT(s->card, dinfo ? dinfo->bdrv : NULL, false);
+    if (dinfo) {
+        object_property_set_int(OBJECT(s->card), dinfo->unit, "if-idx", &errp);
+    }
+    assert_no_error(errp);
+    SD_INIT(s->card, false);
     return 0;
 }
 
diff --git a/hw/pxa.h b/hw/pxa.h
index 025be34..e76f144 100644
--- a/hw/pxa.h
+++ b/hw/pxa.h
@@ -87,7 +87,7 @@ void pxa2xx_lcdc_oritentation(void *opaque, int angle);
 typedef struct PXA2xxMMCIState PXA2xxMMCIState;
 PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
                 target_phys_addr_t base,
-                BlockDriverState *bd, qemu_irq irq,
+                DriveInfo *di, qemu_irq irq,
                 qemu_irq rx_dma, qemu_irq tx_dma);
 void pxa2xx_mmci_handlers(PXA2xxMMCIState *s, qemu_irq readonly,
                 qemu_irq coverswitch);
diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c
index ddaa846..1e5f72f 100644
--- a/hw/pxa2xx.c
+++ b/hw/pxa2xx.c
@@ -14,7 +14,6 @@
 #include "i2c.h"
 #include "ssi.h"
 #include "qemu-char.h"
-#include "blockdev.h"
 
 static struct {
     target_phys_addr_t io_base;
@@ -2101,7 +2100,7 @@ PXA2xxState *pxa270_init(MemoryRegion *address_space,
         fprintf(stderr, "qemu: missing SecureDigital device\n");
         exit(1);
     }
-    s->mmc = pxa2xx_mmci_init(address_space, 0x41100000, dinfo->bdrv,
+    s->mmc = pxa2xx_mmci_init(address_space, 0x41100000, dinfo,
                     qdev_get_gpio_in(s->pic, PXA2XX_PIC_MMC),
                     qdev_get_gpio_in(s->dma, PXA2XX_RX_RQ_MMCI),
                     qdev_get_gpio_in(s->dma, PXA2XX_TX_RQ_MMCI));
@@ -2232,7 +2231,7 @@ PXA2xxState *pxa255_init(MemoryRegion *address_space, unsigned int sdram_size)
         fprintf(stderr, "qemu: missing SecureDigital device\n");
         exit(1);
     }
-    s->mmc = pxa2xx_mmci_init(address_space, 0x41100000, dinfo->bdrv,
+    s->mmc = pxa2xx_mmci_init(address_space, 0x41100000, dinfo,
                     qdev_get_gpio_in(s->pic, PXA2XX_PIC_MMC),
                     qdev_get_gpio_in(s->dma, PXA2XX_RX_RQ_MMCI),
                     qdev_get_gpio_in(s->dma, PXA2XX_TX_RQ_MMCI));
diff --git a/hw/pxa2xx_mmci.c b/hw/pxa2xx_mmci.c
index a7081b0..c06a108 100644
--- a/hw/pxa2xx_mmci.c
+++ b/hw/pxa2xx_mmci.c
@@ -14,6 +14,7 @@
 #include "pxa.h"
 #include "sd.h"
 #include "qdev.h"
+#include "blockdev.h"
 
 struct PXA2xxMMCIState {
     MemoryRegion iomem;
@@ -528,10 +529,11 @@ static int pxa2xx_mmci_load(QEMUFile *f, void *opaque, int version_id)
 
 PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
                 target_phys_addr_t base,
-                BlockDriverState *bd, qemu_irq irq,
+                DriveInfo *di, qemu_irq irq,
                 qemu_irq rx_dma, qemu_irq tx_dma)
 {
     PXA2xxMMCIState *s;
+    Error *errp = NULL;
 
     s = (PXA2xxMMCIState *) g_malloc0(sizeof(PXA2xxMMCIState));
     s->irq = irq;
@@ -544,7 +546,9 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
 
     /* Instantiate the actual storage */
     s->card = SD_CARD(object_new(TYPE_SD_CARD));
-    SD_INIT(s->card, bd, false);
+    object_property_set_int(OBJECT(s->card), di->unit, "if-idx", &errp);
+    assert_no_error(errp);
+    SD_INIT(s->card, false);
 
     register_savevm(NULL, "pxa2xx_mmci", 0, 0,
                     pxa2xx_mmci_save, pxa2xx_mmci_load, s);
diff --git a/hw/sd.c b/hw/sd.c
index 58d94ed..1ea1756 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -30,9 +30,10 @@
  */
 
 #include "hw.h"
-#include "block.h"
+#include "blockdev.h"
 #include "sd.h"
 #include "bitmap.h"
+#include "qapi/qapi-visit-core.h"
 
 //#define DEBUG_SD 1
 
@@ -112,7 +113,7 @@ struct SDState {
     qemu_irq inserted_cb;
     BlockDriverState *bdrv;
     uint8_t *buf;
-
+    int32_t if_idx;
     bool enable;
 };
 
@@ -486,8 +487,14 @@ static const VMStateDescription sd_vmstate = {
    whether card should be in SSI or MMC/SD mode.  It is also up to the
    board to ensure that ssi transfers only occur when the chip select
    is asserted.  */
-static void sd_init(SDState *sd, BlockDriverState *bs, bool is_spi)
+static void sd_init(SDState *sd, bool is_spi)
 {
+    DriveInfo *di = drive_get_by_index(IF_SD, sd->if_idx);
+    BlockDriverState *bs = NULL;
+
+    if (di) {
+        bs = di->bdrv;
+    }
     sd->buf = qemu_blockalign(bs, 512);
     sd->spi = is_spi;
     sd->enable = true;
@@ -1764,10 +1771,44 @@ static void sd_class_init(ObjectClass *class, void *data)
     k->enable = sd_enable;
 }
 
+static void sd_get_if_index(Object *obj, Visitor *v, void *opaque,
+                         const char *name, Error **errp)
+{
+    int64_t if_idx = SD_CARD(obj)->if_idx;
+
+    visit_type_int(v, &if_idx, name, errp);
+}
+
+static void sd_set_if_index(Object *obj, Visitor *v, void *opaque,
+                         const char *name, Error **errp)
+{
+    SDState *sd = SD_CARD(obj);
+    int64_t if_idx;
+
+    visit_type_int(v, &if_idx, name, errp);
+    if (sd->bdrv) {
+        error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(sd->bdrv));
+    } else if (if_idx < 0 || if_idx > INT32_MAX) {
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE, "if-idx", "positive int32");
+    } else {
+        sd->if_idx = if_idx;
+    }
+}
+
+static void sd_initfn(Object *obj)
+{
+    SDState *sd = SD_CARD(obj);
+
+    sd->if_idx = -1;
+    object_property_add(obj, "if-idx", "int", sd_get_if_index, sd_set_if_index,
+            NULL, NULL, NULL);
+}
+
 static TypeInfo sd_type_info = {
     .name = TYPE_SD_CARD,
     .parent = TYPE_OBJECT,
     .instance_size = sizeof(SDState),
+    .instance_init = sd_initfn,
     .class_init = sd_class_init,
     .class_size = sizeof(SDClass)
 };
diff --git a/hw/sd.h b/hw/sd.h
index d8f0195..e2b458a 100644
--- a/hw/sd.h
+++ b/hw/sd.h
@@ -73,7 +73,7 @@ typedef struct SDState SDState;
 typedef struct SDClass {
     ObjectClass parent_class;
 
-    void (*init)(SDState *sd, BlockDriverState *bs, bool is_spi);
+    void (*init)(SDState *sd, bool is_spi);
     int (*do_command)(SDState *sd, SDRequest *req, uint8_t *response);
     void (*write_data)(SDState *sd, uint8_t value);
     uint8_t (*read_data)(SDState *sd);
@@ -90,7 +90,7 @@ typedef struct SDClass {
 #define SD_GET_CLASS(obj)       \
      OBJECT_GET_CLASS(SDClass, (obj), TYPE_SD_CARD)
 
-#define SD_INIT(sd, bdrv, is_spi)   (SD_GET_CLASS(sd)->init(sd, bdrv, is_spi))
+#define SD_INIT(sd, bdrv, is_spi)   (SD_GET_CLASS(sd)->init(sd, is_spi))
 #define SD_DO_COMMAND(sd, req, rsp) (SD_GET_CLASS(sd)->do_command(sd, req, rsp))
 #define SD_WRITE(sd, value)         (SD_GET_CLASS(sd)->write_data(sd, value))
 #define SD_READ(sd)                 (SD_GET_CLASS(sd)->read_data(sd))
diff --git a/hw/ssi-sd.c b/hw/ssi-sd.c
index beecc0e..38057ba 100644
--- a/hw/ssi-sd.c
+++ b/hw/ssi-sd.c
@@ -236,11 +236,16 @@ static int ssi_sd_init(SSISlave *dev)
 {
     ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, dev);
     DriveInfo *dinfo;
+    Error *errp = NULL;
 
     s->mode = SSI_SD_CMD;
     dinfo = drive_get_next(IF_SD);
     s->sd = SD_CARD(object_new(TYPE_SD_CARD));
-    SD_INIT(s->sd, dinfo ? dinfo->bdrv : NULL, true);
+    if (dinfo) {
+        object_property_set_int(OBJECT(s->sd), dinfo->unit, "if-idx", &errp);
+    }
+    assert_no_error(errp);
+    SD_INIT(s->sd, true);
     register_savevm(&dev->qdev, "ssi_sd", -1, 1, ssi_sd_save, ssi_sd_load, s);
     return 0;
 }
-- 
1.7.4.1

[Qemu-devel] [PATCH V2 08/10] SD card: introduce "spi_mode" property for SD card objects

[Igor Mitsyanko]

And drop passing is_spi argument to SDCardClass::init function. After this,
SDCardClass::init is trivialized enough to replace it with Object::realize
callback in the future.

"spi_mode" property should be set before realizing SD card object. It defaults to
"false".

Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
---
 hw/milkymist-memcard.c |    3 ++-
 hw/omap_mmc.c          |    6 ++++--
 hw/pl181.c             |    3 ++-
 hw/pxa2xx_mmci.c       |    3 ++-
 hw/sd.c                |   28 ++++++++++++++++++++++++++--
 hw/sd.h                |    4 ++--
 hw/ssi-sd.c            |    3 ++-
 7 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/hw/milkymist-memcard.c b/hw/milkymist-memcard.c
index 2fff47f..80cac20 100644
--- a/hw/milkymist-memcard.c
+++ b/hw/milkymist-memcard.c
@@ -258,8 +258,9 @@ static int milkymist_memcard_init(SysBusDevice *dev)
     if (dinfo) {
         object_property_set_int(OBJECT(s->card), dinfo->unit, "if-idx", &errp);
     }
+    object_property_set_bool(OBJECT(s->card), false, "spi-mode", &errp);
     assert_no_error(errp);
-    SD_INIT(s->card, false);
+    SD_INIT(s->card);
     s->enabled = dinfo ? bdrv_is_inserted(dinfo->bdrv) : 0;
 
     memory_region_init_io(&s->regs_region, &memcard_mmio_ops, s,
diff --git a/hw/omap_mmc.c b/hw/omap_mmc.c
index 3bf29c2..2c59869 100644
--- a/hw/omap_mmc.c
+++ b/hw/omap_mmc.c
@@ -598,8 +598,9 @@ struct omap_mmc_s *omap_mmc_init(target_phys_addr_t base,
     /* Instantiate the storage */
     s->card = SD_CARD(object_new(TYPE_SD_CARD));
     object_property_set_int(OBJECT(s->card), di->unit, "if-idx", &errp);
+    object_property_set_bool(OBJECT(s->card), false, "spi-mode", &errp);
     assert_no_error(errp);
-    SD_INIT(s->card, false);
+    SD_INIT(s->card);
 
     return s;
 }
@@ -627,8 +628,9 @@ struct omap_mmc_s *omap2_mmc_init(struct omap_target_agent_s *ta,
     /* Instantiate the storage */
     s->card = SD_CARD(object_new(TYPE_SD_CARD));
     object_property_set_int(OBJECT(s->card), di->unit, "if-idx", &errp);
+    object_property_set_bool(OBJECT(s->card), false, "spi-mode", &errp);
     assert_no_error(errp);
-    SD_INIT(s->card, false);
+    SD_INIT(s->card);
 
     s->cdet = qemu_allocate_irqs(omap_mmc_cover_cb, s, 1)[0];
     SD_SET_CB(s->card, NULL, s->cdet);
diff --git a/hw/pl181.c b/hw/pl181.c
index a0a8fe3..48720ae 100644
--- a/hw/pl181.c
+++ b/hw/pl181.c
@@ -489,8 +489,9 @@ static int pl181_init(SysBusDevice *dev)
     if (dinfo) {
         object_property_set_int(OBJECT(s->card), dinfo->unit, "if-idx", &errp);
     }
+    object_property_set_bool(OBJECT(s->card), false, "spi-mode", &errp);
     assert_no_error(errp);
-    SD_INIT(s->card, false);
+    SD_INIT(s->card);
     return 0;
 }
 
diff --git a/hw/pxa2xx_mmci.c b/hw/pxa2xx_mmci.c
index c06a108..1e0cb7b 100644
--- a/hw/pxa2xx_mmci.c
+++ b/hw/pxa2xx_mmci.c
@@ -547,8 +547,9 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
     /* Instantiate the actual storage */
     s->card = SD_CARD(object_new(TYPE_SD_CARD));
     object_property_set_int(OBJECT(s->card), di->unit, "if-idx", &errp);
+    object_property_set_bool(OBJECT(s->card), false, "spi-mode", &errp);
     assert_no_error(errp);
-    SD_INIT(s->card, false);
+    SD_INIT(s->card);
 
     register_savevm(NULL, "pxa2xx_mmci", 0, 0,
                     pxa2xx_mmci_save, pxa2xx_mmci_load, s);
diff --git a/hw/sd.c b/hw/sd.c
index 1ea1756..8ffaa17 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -487,7 +487,7 @@ static const VMStateDescription sd_vmstate = {
    whether card should be in SSI or MMC/SD mode.  It is also up to the
    board to ensure that ssi transfers only occur when the chip select
    is asserted.  */
-static void sd_init(SDState *sd, bool is_spi)
+static void sd_init(SDState *sd)
 {
     DriveInfo *di = drive_get_by_index(IF_SD, sd->if_idx);
     BlockDriverState *bs = NULL;
@@ -496,7 +496,6 @@ static void sd_init(SDState *sd, bool is_spi)
         bs = di->bdrv;
     }
     sd->buf = qemu_blockalign(bs, 512);
-    sd->spi = is_spi;
     sd->enable = true;
     sd_reset(sd, bs);
     if (sd->bdrv) {
@@ -1795,13 +1794,38 @@ static void sd_set_if_index(Object *obj, Visitor *v, void *opaque,
     }
 }
 
+static void sd_is_spi(Object *obj, Visitor *v, void *opaque,
+                         const char *name, Error **errp)
+{
+    SDState *sd = SD_CARD(obj);
+
+    visit_type_bool(v, &sd->spi, name, errp);
+}
+
+static void sd_set_spimode(Object *obj, Visitor *v, void *opaque,
+                         const char *name, Error **errp)
+{
+    SDState *sd = SD_CARD(obj);
+    bool is_spi;
+
+    visit_type_bool(v, &is_spi, name, errp);
+    if (sd->bdrv) {
+        error_set(errp, QERR_DEVICE_IN_USE, bdrv_get_device_name(sd->bdrv));
+    } else {
+        sd->spi = is_spi;
+    }
+}
+
 static void sd_initfn(Object *obj)
 {
     SDState *sd = SD_CARD(obj);
 
     sd->if_idx = -1;
+    sd->spi = false;
     object_property_add(obj, "if-idx", "int", sd_get_if_index, sd_set_if_index,
             NULL, NULL, NULL);
+    object_property_add(obj, "spi-mode", "boolean", sd_is_spi, sd_set_spimode,
+            NULL, NULL, NULL);
 }
 
 static TypeInfo sd_type_info = {
diff --git a/hw/sd.h b/hw/sd.h
index e2b458a..e45ce2f 100644
--- a/hw/sd.h
+++ b/hw/sd.h
@@ -73,7 +73,7 @@ typedef struct SDState SDState;
 typedef struct SDClass {
     ObjectClass parent_class;
 
-    void (*init)(SDState *sd, bool is_spi);
+    void (*init)(SDState *sd);
     int (*do_command)(SDState *sd, SDRequest *req, uint8_t *response);
     void (*write_data)(SDState *sd, uint8_t value);
     uint8_t (*read_data)(SDState *sd);
@@ -90,7 +90,7 @@ typedef struct SDClass {
 #define SD_GET_CLASS(obj)       \
      OBJECT_GET_CLASS(SDClass, (obj), TYPE_SD_CARD)
 
-#define SD_INIT(sd, bdrv, is_spi)   (SD_GET_CLASS(sd)->init(sd, is_spi))
+#define SD_INIT(sd, bdrv, is_spi)   (SD_GET_CLASS(sd)->init(sd))
 #define SD_DO_COMMAND(sd, req, rsp) (SD_GET_CLASS(sd)->do_command(sd, req, rsp))
 #define SD_WRITE(sd, value)         (SD_GET_CLASS(sd)->write_data(sd, value))
 #define SD_READ(sd)                 (SD_GET_CLASS(sd)->read_data(sd))
diff --git a/hw/ssi-sd.c b/hw/ssi-sd.c
index 38057ba..9f4510d 100644
--- a/hw/ssi-sd.c
+++ b/hw/ssi-sd.c
@@ -244,8 +244,9 @@ static int ssi_sd_init(SSISlave *dev)
     if (dinfo) {
         object_property_set_int(OBJECT(s->sd), dinfo->unit, "if-idx", &errp);
     }
+    object_property_set_bool(OBJECT(s->sd), true, "spi-mode", &errp);
     assert_no_error(errp);
-    SD_INIT(s->sd, true);
+    SD_INIT(s->sd);
     register_savevm(&dev->qdev, "ssi_sd", -1, 1, ssi_sd_save, ssi_sd_load, s);
     return 0;
 }
-- 
1.7.4.1

[Qemu-devel] [PATCH V2 09/10] SD card: make SD card a child of host controller

[Igor Mitsyanko]

Only for host controllers implemented as QOM object.

Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
---
 hw/milkymist-memcard.c |    1 +
 hw/pl181.c             |    1 +
 hw/ssi-sd.c            |    1 +
 3 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/milkymist-memcard.c b/hw/milkymist-memcard.c
index 80cac20..c8a6bae 100644
--- a/hw/milkymist-memcard.c
+++ b/hw/milkymist-memcard.c
@@ -259,6 +259,7 @@ static int milkymist_memcard_init(SysBusDevice *dev)
         object_property_set_int(OBJECT(s->card), dinfo->unit, "if-idx", &errp);
     }
     object_property_set_bool(OBJECT(s->card), false, "spi-mode", &errp);
+    object_property_add_child(OBJECT(s), "card", OBJECT(s->card), &errp);
     assert_no_error(errp);
     SD_INIT(s->card);
     s->enabled = dinfo ? bdrv_is_inserted(dinfo->bdrv) : 0;
diff --git a/hw/pl181.c b/hw/pl181.c
index 48720ae..5f81531 100644
--- a/hw/pl181.c
+++ b/hw/pl181.c
@@ -490,6 +490,7 @@ static int pl181_init(SysBusDevice *dev)
         object_property_set_int(OBJECT(s->card), dinfo->unit, "if-idx", &errp);
     }
     object_property_set_bool(OBJECT(s->card), false, "spi-mode", &errp);
+    object_property_add_child(OBJECT(s), "card", OBJECT(s->card), &errp);
     assert_no_error(errp);
     SD_INIT(s->card);
     return 0;
diff --git a/hw/ssi-sd.c b/hw/ssi-sd.c
index 9f4510d..f30a553 100644
--- a/hw/ssi-sd.c
+++ b/hw/ssi-sd.c
@@ -245,6 +245,7 @@ static int ssi_sd_init(SSISlave *dev)
         object_property_set_int(OBJECT(s->sd), dinfo->unit, "if-idx", &errp);
     }
     object_property_set_bool(OBJECT(s->sd), true, "spi-mode", &errp);
+    object_property_add_child(OBJECT(s), "card", OBJECT(s->sd), &errp);
     assert_no_error(errp);
     SD_INIT(s->sd);
     register_savevm(&dev->qdev, "ssi_sd", -1, 1, ssi_sd_save, ssi_sd_load, s);
-- 
1.7.4.1

[Qemu-devel] [PATCH V2 10/10] hw/sd.c: introduce SD card "image" property and allow SD hot-insert

[Igor Mitsyanko]

New SD card "image" property can be used to:
- change image file attached to virtual SD card
- hot-insert new image file into newly initialized BlockDriverState (this was not
possible before).

Example usage:
./qom-set /machine/milkymist/milkymist-memcard/card.image /home/me/mynewcard.img
this will attach image file /home/me/mynewcard.img to virtual SD card connected to
milkymist-memcard host controller device. If virtual card was already attached to
some other image file, eject event is triggered before attaching new file.

Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
---
 hw/sd.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index 8ffaa17..3e75405 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -1816,6 +1816,48 @@ static void sd_set_spimode(Object *obj, Visitor *v, void *opaque,
     }
 }
 
+static void sd_set_image_path(Object *obj, Visitor *v, void *opaque,
+                         const char *name, Error **errp)
+{
+    SDState *sd = SD_CARD(obj);
+    char *new_image;
+
+    visit_type_str(v, &new_image, "file", errp);
+
+    if (error_is_set(errp)) {
+        return;
+    }
+
+    if (sd->bdrv) {
+        qmp_change_blockdev(bdrv_get_device_name(sd->bdrv), new_image,
+                false, NULL, errp);
+    } else {
+        DriveInfo *di;
+        QemuOpts *opts = drive_add(IF_SD, sd->if_idx, new_image, "");
+
+        if (!opts) {
+            error_set(errp, QERR_OPEN_FILE_FAILED, new_image);
+            return;
+        }
+
+        di = drive_init(opts, 0);
+        if (!di) {
+            error_set(errp, QERR_OPEN_FILE_FAILED, new_image);
+            return;
+        }
+
+        sd_reset(sd, di->bdrv);
+        if (bdrv_attach_dev(sd->bdrv, sd) < 0) {
+            drive_put_ref(di);
+            error_set(errp, QERR_OPEN_FILE_FAILED, new_image);
+            return;
+        }
+        bdrv_set_dev_ops(sd->bdrv, &sd_block_ops, sd);
+        qemu_set_irq(sd->inserted_cb, bdrv_is_inserted(sd->bdrv));
+        qemu_set_irq(sd->readonly_cb, sd->wp_switch);
+    }
+}
+
 static void sd_initfn(Object *obj)
 {
     SDState *sd = SD_CARD(obj);
@@ -1826,6 +1868,8 @@ static void sd_initfn(Object *obj)
             NULL, NULL, NULL);
     object_property_add(obj, "spi-mode", "boolean", sd_is_spi, sd_set_spimode,
             NULL, NULL, NULL);
+    object_property_add(OBJECT(sd), "image", "string",
+            NULL, sd_set_image_path, NULL, NULL, NULL);
 }
 
 static TypeInfo sd_type_info = {
-- 
1.7.4.1

Re: [Qemu-devel] [PATCH V2 10/10] hw/sd.c: introduce SD card "image" property and allow SD hot-insert

[Paolo Bonzini]

Il 05/04/2012 17:48, Igor Mitsyanko ha scritto:
> New SD card "image" property can be used to:
> - change image file attached to virtual SD card
> - hot-insert new image file into newly initialized BlockDriverState (this was not
> possible before).
> 
> Example usage:
> ./qom-set /machine/milkymist/milkymist-memcard/card.image /home/me/mynewcard.img
> this will attach image file /home/me/mynewcard.img to virtual SD card connected to
> milkymist-memcard host controller device. If virtual card was already attached to
> some other image file, eject event is triggered before attaching new file.
> 
> Signed-off-by: Igor Mitsyanko <i.mitsyanko@samsung.com>
> ---
>  hw/sd.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 44 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/sd.c b/hw/sd.c
> index 8ffaa17..3e75405 100644
> --- a/hw/sd.c
> +++ b/hw/sd.c
> @@ -1816,6 +1816,48 @@ static void sd_set_spimode(Object *obj, Visitor *v, void *opaque,
>      }
>  }
>  
> +static void sd_set_image_path(Object *obj, Visitor *v, void *opaque,
> +                         const char *name, Error **errp)
> +{
> +    SDState *sd = SD_CARD(obj);
> +    char *new_image;
> +
> +    visit_type_str(v, &new_image, "file", errp);

Please use the name argument instead of "file".

> +
> +    if (error_is_set(errp)) {
> +        return;
> +    }
> +
> +    if (sd->bdrv) {
> +        qmp_change_blockdev(bdrv_get_device_name(sd->bdrv), new_image,
> +                false, NULL, errp);
> +    } else {
> +        DriveInfo *di;
> +        QemuOpts *opts = drive_add(IF_SD, sd->if_idx, new_image, "");

I think this should simply take a drive name and pass it to
bdrv_get_device_name.  The drive_add/drive_init should be done
separately with the drive_add monitor command, like

drive_add 0 file=foo.img,if=none,id=bar

With the upcoming support for static properties in objects that are not
devices, you can just add a drive property to the SD class.

There is a problem here however.  QOM commands are HMP only, and
drive_add is QMP only.  I think that blocking drive_add in QMP is
perfect being the enemy of good.  Alternatively, however, adding the QOM
property commands to HMP would also make sense.

Paolo