[Qemu-devel] [PATCH 0/6] SDHCI: convert legacy devices to the SDBus API

[Qemu-devel] [PATCH 0/6] SDHCI: convert legacy devices to the SDBus API

[Philippe Mathieu-Daudé]

Hi,

This series convert 3 devices using the legacy SDCard API to the SDBus API:
- milkymist-mmc
- pl181
- ssi-sd

Then move the legacy API to a separate header "sdcard_legacy.h".

Now the OMAP MMC is the last device using the legacy API, but need to get
QOM'ified first.

Regards,

Phil.

Philippe Mathieu-Daudé (6):
  hw/sd/milkymist-memcard: use qemu_log_mask()
  hw/sd/milkymist-memcard: split realize() out of SysBusDevice init()
  hw/sd/milkymist-memcard: expose a SDBus and connect the SDCard to it
  hw/sd/pl181: expose a SDBus and connect the SDCard to it
  hw/sd/ssi-sd: expose a SDBus and connect the SDCard to it
  hw/sd: move sdcard legacy API to "hw/sd/sdcard_legacy.h"

 include/hw/sd/sd.h            | 17 ----------
 include/hw/sd/sdcard_legacy.h | 51 ++++++++++++++++++++++++++++++
 hw/sd/milkymist-memcard.c     | 72 ++++++++++++++++++++++++++-----------------
 hw/sd/omap_mmc.c              |  2 +-
 hw/sd/pl181.c                 | 28 ++++++++++++-----
 hw/sd/sd.c                    |  1 +
 hw/sd/ssi-sd.c                | 27 +++++++++++-----
 7 files changed, 137 insertions(+), 61 deletions(-)
 create mode 100644 include/hw/sd/sdcard_legacy.h

-- 
2.15.1

[Qemu-devel] [PATCH 1/6] hw/sd/milkymist-memcard: use qemu_log_mask()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/milkymist-memcard.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 4008c81002..5de3e00e2f 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -26,7 +26,7 @@
 #include "hw/sysbus.h"
 #include "sysemu/sysemu.h"
 #include "trace.h"
-#include "qemu/error-report.h"
+#include "include/qapi/error.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
 #include "hw/sd/sd.h"
@@ -138,8 +138,8 @@ static uint64_t memcard_read(void *opaque, hwaddr addr,
         } else {
             r = s->response[s->response_read_ptr++];
             if (s->response_read_ptr > s->response_len) {
-                error_report("milkymist_memcard: "
-                        "read more cmd bytes than available. Clipping.");
+                qemu_log_mask(LOG_GUEST_ERROR, "milkymist_memcard: "
+                              "read more cmd bytes than available. Clipping.");
                 s->response_read_ptr = 0;
             }
         }
@@ -163,8 +163,9 @@ static uint64_t memcard_read(void *opaque, hwaddr addr,
         break;
 
     default:
-        error_report("milkymist_memcard: read access to unknown register 0x"
-                TARGET_FMT_plx, addr << 2);
+        qemu_log_mask(LOG_UNIMP, "milkymist_memcard: "
+                      "read access to unknown register 0x%" HWADDR_PRIx "\n",
+                      addr << 2);
         break;
     }
 
@@ -220,8 +221,9 @@ static void memcard_write(void *opaque, hwaddr addr, uint64_t value,
         break;
 
     default:
-        error_report("milkymist_memcard: write access to unknown register 0x"
-                TARGET_FMT_plx, addr << 2);
+        qemu_log_mask(LOG_UNIMP, "milkymist_memcard: "
+                      "write access to unknown register 0x%" HWADDR_PRIx " "
+                      "(value 0x%" PRIx64 ")\n", addr << 2, value);
         break;
     }
 }
-- 
2.15.1

[Qemu-devel] [PATCH 2/6] hw/sd/milkymist-memcard: split realize() out of SysBusDevice init()

[Philippe Mathieu-Daudé]

Create the SDCard in the realize() function.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/milkymist-memcard.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 5de3e00e2f..5df3a0f815 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -255,24 +255,29 @@ static void milkymist_memcard_reset(DeviceState *d)
 static int milkymist_memcard_init(SysBusDevice *dev)
 {
     MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
-    DriveInfo *dinfo;
+
+    memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s,
+            "milkymist-memcard", R_MAX * 4);
+    sysbus_init_mmio(dev, &s->regs_region);
+
+    return 0;
+}
+
+static void milkymist_memcard_realize(DeviceState *dev, Error **errp)
+{
+    MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
     BlockBackend *blk;
+    DriveInfo *dinfo;
 
     /* FIXME use a qdev drive property instead of drive_get_next() */
     dinfo = drive_get_next(IF_SD);
     blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
     s->card = sd_init(blk, false);
     if (s->card == NULL) {
-        return -1;
+        error_setg(errp, "failed to init SD card");
+        return;
     }
-
     s->enabled = blk && blk_is_inserted(blk);
-
-    memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s,
-            "milkymist-memcard", R_MAX * 4);
-    sysbus_init_mmio(dev, &s->regs_region);
-
-    return 0;
 }
 
 static const VMStateDescription vmstate_milkymist_memcard = {
@@ -298,6 +303,7 @@ static void milkymist_memcard_class_init(ObjectClass *klass, void *data)
     SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
 
     k->init = milkymist_memcard_init;
+    dc->realize = milkymist_memcard_realize;
     dc->reset = milkymist_memcard_reset;
     dc->vmsd = &vmstate_milkymist_memcard;
     /* Reason: init() method uses drive_get_next() */
-- 
2.15.1

[Qemu-devel] [PATCH 3/6] hw/sd/milkymist-memcard: expose a SDBus and connect the SDCard to it

[Philippe Mathieu-Daudé]

using the sdbus_*() API.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/milkymist-memcard.c | 34 +++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 5df3a0f815..9f4c7dad63 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -68,7 +68,7 @@ struct MilkymistMemcardState {
     SysBusDevice parent_obj;
 
     MemoryRegion regs_region;
-    SDState *card;
+    SDBus sdbus;
 
     int command_write_ptr;
     int response_read_ptr;
@@ -104,7 +104,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 = sdbus_do_command(&s->sdbus, &req, s->response + 1);
     s->response_read_ptr = 0;
 
     if (s->response_len == 16) {
@@ -149,10 +149,10 @@ static uint64_t memcard_read(void *opaque, hwaddr addr,
             r = 0xffffffff;
         } else {
             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 |= sdbus_read_data(&s->sdbus) << 24;
+            r |= sdbus_read_data(&s->sdbus) << 16;
+            r |= sdbus_read_data(&s->sdbus) << 8;
+            r |= sdbus_read_data(&s->sdbus);
         }
         break;
     case R_CLK2XDIV:
@@ -206,10 +206,10 @@ static void memcard_write(void *opaque, hwaddr 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);
+        sdbus_write_data(&s->sdbus, (value >> 24) & 0xff);
+        sdbus_write_data(&s->sdbus, (value >> 16) & 0xff);
+        sdbus_write_data(&s->sdbus, (value >> 8) & 0xff);
+        sdbus_write_data(&s->sdbus, value & 0xff);
         break;
     case R_ENABLE:
         s->regs[addr] = value;
@@ -266,15 +266,23 @@ static int milkymist_memcard_init(SysBusDevice *dev)
 static void milkymist_memcard_realize(DeviceState *dev, Error **errp)
 {
     MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
+    DeviceState *carddev;
     BlockBackend *blk;
     DriveInfo *dinfo;
+    Error *err = NULL;
 
+    qbus_create_inplace(&s->sdbus, sizeof(s->sdbus), TYPE_SD_BUS,
+                        dev, "sd-bus");
+
+    /* Create and plug in the sd card */
     /* FIXME use a qdev drive property instead of drive_get_next() */
     dinfo = drive_get_next(IF_SD);
     blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
-    s->card = sd_init(blk, false);
-    if (s->card == NULL) {
-        error_setg(errp, "failed to init SD card");
+    carddev = qdev_create(&s->sdbus.qbus, TYPE_SD_CARD);
+    qdev_prop_set_drive(carddev, "drive", blk, &err);
+    object_property_set_bool(OBJECT(carddev), true, "realized", &err);
+    if (err) {
+        error_setg(errp, "failed to init SD card: %s", error_get_pretty(err));
         return;
     }
     s->enabled = blk && blk_is_inserted(blk);
-- 
2.15.1

[Qemu-devel] [PATCH 4/6] hw/sd/pl181: expose a SDBus and connect the SDCard to it

[Philippe Mathieu-Daudé]

using the sdbus_*() API.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/pl181.c | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index 55c8098ecd..ce696c5d7d 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -33,6 +33,7 @@ typedef struct PL181State {
     SysBusDevice parent_obj;
 
     MemoryRegion iomem;
+    SDBus sdbus;
     SDState *card;
     uint32_t clock;
     uint32_t power;
@@ -179,7 +180,7 @@ static void pl181_send_command(PL181State *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 = sdbus_do_command(&s->sdbus, &request, response);
     if (rlen < 0)
         goto error;
     if (s->cmd & PL181_CMD_RESPONSE) {
@@ -223,12 +224,12 @@ static void pl181_fifo_run(PL181State *s)
     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 || sdbus_data_ready(&s->sdbus))
             && !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)sdbus_read_data(&s->sdbus) << (n * 8);
                 s->datacnt--;
                 n++;
                 if (n == 4) {
@@ -249,7 +250,7 @@ static void pl181_fifo_run(PL181State *s)
                 }
                 n--;
                 s->datacnt--;
-                sd_write_data(s->card, value & 0xff);
+                sdbus_write_data(&s->sdbus, value & 0xff);
                 value >>= 8;
             }
         }
@@ -498,14 +499,26 @@ static void pl181_init(Object *obj)
 static void pl181_realize(DeviceState *dev, Error **errp)
 {
     PL181State *s = PL181(dev);
+    DeviceState *carddev;
     DriveInfo *dinfo;
+    Error *err = NULL;
 
+    qbus_create_inplace(&s->sdbus, sizeof(s->sdbus), TYPE_SD_BUS,
+                        dev, "sd-bus");
+
+    /* Create and plug in the sd card */
     /* FIXME use a qdev drive property instead of drive_get_next() */
     dinfo = drive_get_next(IF_SD);
-    s->card = sd_init(dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, false);
-    if (s->card == NULL) {
-        error_setg(errp, "sd_init failed");
+    carddev = qdev_create(&s->sdbus.qbus, TYPE_SD_CARD);
+    if (dinfo) {
+        qdev_prop_set_drive(carddev, "drive", blk_by_legacy_dinfo(dinfo), &err);
+    }
+    object_property_set_bool(OBJECT(carddev), true, "realized", &err);
+    if (err) {
+        error_setg(errp, "failed to init SD card: %s", error_get_pretty(err));
+        return;
     }
+    s->card = SD_CARD(carddev);
 }
 
 static void pl181_class_init(ObjectClass *klass, void *data)
-- 
2.15.1

[Qemu-devel] [PATCH 5/6] hw/sd/ssi-sd: expose a SDBus and connect the SDCard to it

[Philippe Mathieu-Daudé]

using the sdbus_*() API.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/ssi-sd.c | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 24001dc3e6..c8b27add84 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -47,7 +47,7 @@ typedef struct {
     int32_t arglen;
     int32_t response_pos;
     int32_t stopping;
-    SDState *sd;
+    SDBus sdbus;
 } ssi_sd_state;
 
 /* State word bits.  */
@@ -97,7 +97,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 = sdbus_do_command(&s->sdbus, &request, longresp);
             if (s->arglen <= 0) {
                 s->arglen = 1;
                 s->response[0] = 4;
@@ -174,7 +174,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 (sdbus_data_ready(&s->sdbus)) {
             DPRINTF("Data read\n");
             s->mode = SSI_SD_DATA_START;
         } else {
@@ -187,8 +187,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 = sdbus_read_data(&s->sdbus);
+        if (!sdbus_data_ready(&s->sdbus)) {
             DPRINTF("Data read end\n");
             s->mode = SSI_SD_CMD;
         }
@@ -239,14 +239,25 @@ static const VMStateDescription vmstate_ssi_sd = {
 static void ssi_sd_realize(SSISlave *d, Error **errp)
 {
     ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, d);
+    DeviceState *carddev;
     DriveInfo *dinfo;
+    Error *err = NULL;
 
     s->mode = SSI_SD_CMD;
+    qbus_create_inplace(&s->sdbus, sizeof(s->sdbus), TYPE_SD_BUS,
+                        DEVICE(d), "sd-bus");
+
+    /* Create and plug in the sd card */
     /* FIXME use a qdev drive property instead of drive_get_next() */
     dinfo = drive_get_next(IF_SD);
-    s->sd = sd_init(dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, true);
-    if (s->sd == NULL) {
-        error_setg(errp, "Device initialization failed.");
+    carddev = qdev_create(&s->sdbus.qbus, TYPE_SD_CARD);
+    if (dinfo) {
+        qdev_prop_set_drive(carddev, "drive", blk_by_legacy_dinfo(dinfo), &err);
+    }
+    object_property_set_bool(OBJECT(carddev), true, "spi", &err);
+    object_property_set_bool(OBJECT(carddev), true, "realized", &err);
+    if (err) {
+        error_setg(errp, "failed to init SD card: %s", error_get_pretty(err));
         return;
     }
 }
-- 
2.15.1

[Qemu-devel] [PATCH 6/6] hw/sd: move sdcard legacy API to "hw/sd/sdcard_legacy.h"

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
roughly 2 users left.

 include/hw/sd/sd.h            | 17 ---------------
 include/hw/sd/sdcard_legacy.h | 51 +++++++++++++++++++++++++++++++++++++++++++
 hw/sd/omap_mmc.c              |  2 +-
 hw/sd/pl181.c                 |  1 +
 hw/sd/sd.c                    |  1 +
 5 files changed, 54 insertions(+), 18 deletions(-)
 create mode 100644 include/hw/sd/sdcard_legacy.h

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 96caefe373..a342e7bc3e 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -114,23 +114,6 @@ typedef struct {
     void (*set_readonly)(DeviceState *dev, bool readonly);
 } SDBusClass;
 
-/* Legacy functions to be used only by non-qdevified callers */
-SDState *sd_init(BlockBackend *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);
-/* sd_enable should not be used -- it is only used on the nseries boards,
- * where it is part of a broken implementation of the MMC card slot switch
- * (there should be two card slots which are multiplexed to a single MMC
- * controller, but instead we model it with one card and controller and
- * disable the card when the second slot is selected, so it looks like the
- * second slot is always empty).
- */
-void sd_enable(SDState *sd, bool enable);
-
 /* Functions to be used by qdevified callers (working via
  * an SDBus rather than directly with SDState)
  */
diff --git a/include/hw/sd/sdcard_legacy.h b/include/hw/sd/sdcard_legacy.h
new file mode 100644
index 0000000000..882e13a8f1
--- /dev/null
+++ b/include/hw/sd/sdcard_legacy.h
@@ -0,0 +1,51 @@
+/*
+ * SD Memory Card emulation (deprecated legacy API)
+ *
+ * Copyright (c) 2006 Andrzej Zaborowski  <balrog@zabor.org>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in
+ *    the documentation and/or other materials provided with the
+ *    distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
+ * PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+#ifndef HW_SDCARD_LEGACY_H
+#define HW_SDCARD_LEGACY_H
+
+#include "hw/sd/sd.h"
+
+/* Legacy functions to be used only by non-qdevified callers */
+SDState *sd_init(BlockBackend *blk, bool is_spi);
+int sd_do_command(SDState *card, SDRequest *request, uint8_t *response);
+void sd_write_data(SDState *card, uint8_t value);
+uint8_t sd_read_data(SDState *card);
+void sd_set_cb(SDState *card, qemu_irq readonly, qemu_irq insert);
+bool sd_data_ready(SDState *card);
+
+/* sd_enable should not be used -- it is only used on the nseries boards,
+ * where it is part of a broken implementation of the MMC card slot switch
+ * (there should be two card slots which are multiplexed to a single MMC
+ * controller, but instead we model it with one card and controller and
+ * disable the card when the second slot is selected, so it looks like the
+ * second slot is always empty).
+ */
+void sd_enable(SDState *card, bool enable);
+
+#endif /* HW_SDCARD_LEGACY_H */
diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
index e934cd3656..9cfdc7a6ad 100644
--- a/hw/sd/omap_mmc.c
+++ b/hw/sd/omap_mmc.c
@@ -19,7 +19,7 @@
 #include "qemu/osdep.h"
 #include "hw/hw.h"
 #include "hw/arm/omap.h"
-#include "hw/sd/sd.h"
+#include "hw/sd/sdcard_legacy.h"
 
 struct omap_mmc_s {
     qemu_irq irq;
diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index ce696c5d7d..7591d016cd 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -12,6 +12,7 @@
 #include "sysemu/blockdev.h"
 #include "hw/sysbus.h"
 #include "hw/sd/sd.h"
+#include "hw/sd/sdcard_legacy.h"
 #include "qemu/log.h"
 #include "qapi/error.h"
 
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 35347a5bbc..7755bedfa0 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -34,6 +34,7 @@
 #include "hw/hw.h"
 #include "sysemu/block-backend.h"
 #include "hw/sd/sd.h"
+#include "hw/sd/sdcard_legacy.h"
 #include "qapi/error.h"
 #include "qemu/bitmap.h"
 #include "hw/qdev-properties.h"
-- 
2.15.1

Re: [Qemu-devel] [PATCH 1/6] hw/sd/milkymist-memcard: use qemu_log_mask()

[Alistair Francis]

On Wed, Jan 3, 2018 at 8:23 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/sd/milkymist-memcard.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
> index 4008c81002..5de3e00e2f 100644
> --- a/hw/sd/milkymist-memcard.c
> +++ b/hw/sd/milkymist-memcard.c
> @@ -26,7 +26,7 @@
>  #include "hw/sysbus.h"
>  #include "sysemu/sysemu.h"
>  #include "trace.h"
> -#include "qemu/error-report.h"
> +#include "include/qapi/error.h"
>  #include "sysemu/block-backend.h"
>  #include "sysemu/blockdev.h"
>  #include "hw/sd/sd.h"
> @@ -138,8 +138,8 @@ static uint64_t memcard_read(void *opaque, hwaddr addr,
>          } else {
>              r = s->response[s->response_read_ptr++];
>              if (s->response_read_ptr > s->response_len) {
> -                error_report("milkymist_memcard: "
> -                        "read more cmd bytes than available. Clipping.");
> +                qemu_log_mask(LOG_GUEST_ERROR, "milkymist_memcard: "
> +                              "read more cmd bytes than available. Clipping.");
>                  s->response_read_ptr = 0;
>              }
>          }
> @@ -163,8 +163,9 @@ static uint64_t memcard_read(void *opaque, hwaddr addr,
>          break;
>
>      default:
> -        error_report("milkymist_memcard: read access to unknown register 0x"
> -                TARGET_FMT_plx, addr << 2);
> +        qemu_log_mask(LOG_UNIMP, "milkymist_memcard: "
> +                      "read access to unknown register 0x%" HWADDR_PRIx "\n",
> +                      addr << 2);
>          break;
>      }
>
> @@ -220,8 +221,9 @@ static void memcard_write(void *opaque, hwaddr addr, uint64_t value,
>          break;
>
>      default:
> -        error_report("milkymist_memcard: write access to unknown register 0x"
> -                TARGET_FMT_plx, addr << 2);
> +        qemu_log_mask(LOG_UNIMP, "milkymist_memcard: "
> +                      "write access to unknown register 0x%" HWADDR_PRIx " "
> +                      "(value 0x%" PRIx64 ")\n", addr << 2, value);

Can you fix up the secondary lines indentation?

After that:

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair


>          break;
>      }
>  }
> --
> 2.15.1
>
>

Re: [Qemu-devel] [PATCH 1/6] hw/sd/milkymist-memcard: use qemu_log_mask()

[Philippe Mathieu-Daudé]

I'm too comfortable with the 'log' trace backend and again forgot to add
the "qemu/log.h" dependency ...

/home/travis/build/philmd/qemu/hw/sd/milkymist-memcard.c:141:17: error:
implicit declaration of function ‘qemu_log_mask’
[-Werror=implicit-function-declaration]
                 qemu_log_mask(LOG_GUEST_ERROR, "milkymist_memcard: "
                 ^
hw/sd/milkymist-memcard.c:141:31: error: ‘LOG_GUEST_ERROR’ undeclared
(first use in this function)
                 qemu_log_mask(LOG_GUEST_ERROR, "milkymist_memcard: "
                               ^
hw/sd/milkymist-memcard.c:166:23: error: ‘LOG_UNIMP’ undeclared (first
use in this function)
         qemu_log_mask(LOG_UNIMP, "milkymist_memcard: "
                       ^

On 01/03/2018 01:23 PM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/sd/milkymist-memcard.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
> index 4008c81002..5de3e00e2f 100644
> --- a/hw/sd/milkymist-memcard.c
> +++ b/hw/sd/milkymist-memcard.c
> @@ -26,7 +26,7 @@
>  #include "hw/sysbus.h"
>  #include "sysemu/sysemu.h"
>  #include "trace.h"
> -#include "qemu/error-report.h"
> +#include "include/qapi/error.h"
>  #include "sysemu/block-backend.h"
>  #include "sysemu/blockdev.h"
>  #include "hw/sd/sd.h"
> @@ -138,8 +138,8 @@ static uint64_t memcard_read(void *opaque, hwaddr addr,
>          } else {
>              r = s->response[s->response_read_ptr++];
>              if (s->response_read_ptr > s->response_len) {
> -                error_report("milkymist_memcard: "
> -                        "read more cmd bytes than available. Clipping.");
> +                qemu_log_mask(LOG_GUEST_ERROR, "milkymist_memcard: "
> +                              "read more cmd bytes than available. Clipping.");
>                  s->response_read_ptr = 0;
>              }
>          }
> @@ -163,8 +163,9 @@ static uint64_t memcard_read(void *opaque, hwaddr addr,
>          break;
>  
>      default:
> -        error_report("milkymist_memcard: read access to unknown register 0x"
> -                TARGET_FMT_plx, addr << 2);
> +        qemu_log_mask(LOG_UNIMP, "milkymist_memcard: "
> +                      "read access to unknown register 0x%" HWADDR_PRIx "\n",
> +                      addr << 2);
>          break;
>      }
>  
> @@ -220,8 +221,9 @@ static void memcard_write(void *opaque, hwaddr addr, uint64_t value,
>          break;
>  
>      default:
> -        error_report("milkymist_memcard: write access to unknown register 0x"
> -                TARGET_FMT_plx, addr << 2);
> +        qemu_log_mask(LOG_UNIMP, "milkymist_memcard: "
> +                      "write access to unknown register 0x%" HWADDR_PRIx " "
> +                      "(value 0x%" PRIx64 ")\n", addr << 2, value);
>          break;
>      }
>  }
> 

Re: [Qemu-devel] [PATCH 2/6] hw/sd/milkymist-memcard: split realize() out of SysBusDevice init()

[Alistair Francis]

On Wed, Jan 3, 2018 at 8:23 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Create the SDCard in the realize() function.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/milkymist-memcard.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
> index 5de3e00e2f..5df3a0f815 100644
> --- a/hw/sd/milkymist-memcard.c
> +++ b/hw/sd/milkymist-memcard.c
> @@ -255,24 +255,29 @@ static void milkymist_memcard_reset(DeviceState *d)
>  static int milkymist_memcard_init(SysBusDevice *dev)
>  {
>      MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
> -    DriveInfo *dinfo;
> +
> +    memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s,
> +            "milkymist-memcard", R_MAX * 4);
> +    sysbus_init_mmio(dev, &s->regs_region);
> +
> +    return 0;
> +}
> +
> +static void milkymist_memcard_realize(DeviceState *dev, Error **errp)
> +{
> +    MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
>      BlockBackend *blk;
> +    DriveInfo *dinfo;
>
>      /* FIXME use a qdev drive property instead of drive_get_next() */
>      dinfo = drive_get_next(IF_SD);
>      blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
>      s->card = sd_init(blk, false);
>      if (s->card == NULL) {
> -        return -1;
> +        error_setg(errp, "failed to init SD card");
> +        return;
>      }
> -
>      s->enabled = blk && blk_is_inserted(blk);
> -
> -    memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s,
> -            "milkymist-memcard", R_MAX * 4);
> -    sysbus_init_mmio(dev, &s->regs_region);
> -
> -    return 0;
>  }
>
>  static const VMStateDescription vmstate_milkymist_memcard = {
> @@ -298,6 +303,7 @@ static void milkymist_memcard_class_init(ObjectClass *klass, void *data)
>      SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>
>      k->init = milkymist_memcard_init;
> +    dc->realize = milkymist_memcard_realize;
>      dc->reset = milkymist_memcard_reset;
>      dc->vmsd = &vmstate_milkymist_memcard;
>      /* Reason: init() method uses drive_get_next() */
> --
> 2.15.1
>
>

Re: [Qemu-devel] [PATCH 6/6] hw/sd: move sdcard legacy API to "hw/sd/sdcard_legacy.h"

[Alistair Francis]

On Wed, Jan 3, 2018 at 8:24 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> roughly 2 users left.

Exciting!

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair


>
>  include/hw/sd/sd.h            | 17 ---------------
>  include/hw/sd/sdcard_legacy.h | 51 +++++++++++++++++++++++++++++++++++++++++++
>  hw/sd/omap_mmc.c              |  2 +-
>  hw/sd/pl181.c                 |  1 +
>  hw/sd/sd.c                    |  1 +
>  5 files changed, 54 insertions(+), 18 deletions(-)
>  create mode 100644 include/hw/sd/sdcard_legacy.h
>
> diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
> index 96caefe373..a342e7bc3e 100644
> --- a/include/hw/sd/sd.h
> +++ b/include/hw/sd/sd.h
> @@ -114,23 +114,6 @@ typedef struct {
>      void (*set_readonly)(DeviceState *dev, bool readonly);
>  } SDBusClass;
>
> -/* Legacy functions to be used only by non-qdevified callers */
> -SDState *sd_init(BlockBackend *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);
> -/* sd_enable should not be used -- it is only used on the nseries boards,
> - * where it is part of a broken implementation of the MMC card slot switch
> - * (there should be two card slots which are multiplexed to a single MMC
> - * controller, but instead we model it with one card and controller and
> - * disable the card when the second slot is selected, so it looks like the
> - * second slot is always empty).
> - */
> -void sd_enable(SDState *sd, bool enable);
> -
>  /* Functions to be used by qdevified callers (working via
>   * an SDBus rather than directly with SDState)
>   */
> diff --git a/include/hw/sd/sdcard_legacy.h b/include/hw/sd/sdcard_legacy.h
> new file mode 100644
> index 0000000000..882e13a8f1
> --- /dev/null
> +++ b/include/hw/sd/sdcard_legacy.h
> @@ -0,0 +1,51 @@
> +/*
> + * SD Memory Card emulation (deprecated legacy API)
> + *
> + * Copyright (c) 2006 Andrzej Zaborowski  <balrog@zabor.org>
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + *
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in
> + *    the documentation and/or other materials provided with the
> + *    distribution.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS''
> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
> + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> + * PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR
> + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
> + * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> + * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
> + * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
> + * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +#ifndef HW_SDCARD_LEGACY_H
> +#define HW_SDCARD_LEGACY_H
> +
> +#include "hw/sd/sd.h"
> +
> +/* Legacy functions to be used only by non-qdevified callers */
> +SDState *sd_init(BlockBackend *blk, bool is_spi);
> +int sd_do_command(SDState *card, SDRequest *request, uint8_t *response);
> +void sd_write_data(SDState *card, uint8_t value);
> +uint8_t sd_read_data(SDState *card);
> +void sd_set_cb(SDState *card, qemu_irq readonly, qemu_irq insert);
> +bool sd_data_ready(SDState *card);
> +
> +/* sd_enable should not be used -- it is only used on the nseries boards,
> + * where it is part of a broken implementation of the MMC card slot switch
> + * (there should be two card slots which are multiplexed to a single MMC
> + * controller, but instead we model it with one card and controller and
> + * disable the card when the second slot is selected, so it looks like the
> + * second slot is always empty).
> + */
> +void sd_enable(SDState *card, bool enable);
> +
> +#endif /* HW_SDCARD_LEGACY_H */
> diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
> index e934cd3656..9cfdc7a6ad 100644
> --- a/hw/sd/omap_mmc.c
> +++ b/hw/sd/omap_mmc.c
> @@ -19,7 +19,7 @@
>  #include "qemu/osdep.h"
>  #include "hw/hw.h"
>  #include "hw/arm/omap.h"
> -#include "hw/sd/sd.h"
> +#include "hw/sd/sdcard_legacy.h"
>
>  struct omap_mmc_s {
>      qemu_irq irq;
> diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
> index ce696c5d7d..7591d016cd 100644
> --- a/hw/sd/pl181.c
> +++ b/hw/sd/pl181.c
> @@ -12,6 +12,7 @@
>  #include "sysemu/blockdev.h"
>  #include "hw/sysbus.h"
>  #include "hw/sd/sd.h"
> +#include "hw/sd/sdcard_legacy.h"
>  #include "qemu/log.h"
>  #include "qapi/error.h"
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 35347a5bbc..7755bedfa0 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -34,6 +34,7 @@
>  #include "hw/hw.h"
>  #include "sysemu/block-backend.h"
>  #include "hw/sd/sd.h"
> +#include "hw/sd/sdcard_legacy.h"
>  #include "qapi/error.h"
>  #include "qemu/bitmap.h"
>  #include "hw/qdev-properties.h"
> --
> 2.15.1
>
>

Re: [Qemu-devel] [PATCH 3/6] hw/sd/milkymist-memcard: expose a SDBus and connect the SDCard to it

[Alistair Francis]

On Wed, Jan 3, 2018 at 8:23 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> using the sdbus_*() API.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/milkymist-memcard.c | 34 +++++++++++++++++++++-------------
>  1 file changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
> index 5df3a0f815..9f4c7dad63 100644
> --- a/hw/sd/milkymist-memcard.c
> +++ b/hw/sd/milkymist-memcard.c
> @@ -68,7 +68,7 @@ struct MilkymistMemcardState {
>      SysBusDevice parent_obj;
>
>      MemoryRegion regs_region;
> -    SDState *card;
> +    SDBus sdbus;
>
>      int command_write_ptr;
>      int response_read_ptr;
> @@ -104,7 +104,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 = sdbus_do_command(&s->sdbus, &req, s->response + 1);
>      s->response_read_ptr = 0;
>
>      if (s->response_len == 16) {
> @@ -149,10 +149,10 @@ static uint64_t memcard_read(void *opaque, hwaddr addr,
>              r = 0xffffffff;
>          } else {
>              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 |= sdbus_read_data(&s->sdbus) << 24;
> +            r |= sdbus_read_data(&s->sdbus) << 16;
> +            r |= sdbus_read_data(&s->sdbus) << 8;
> +            r |= sdbus_read_data(&s->sdbus);
>          }
>          break;
>      case R_CLK2XDIV:
> @@ -206,10 +206,10 @@ static void memcard_write(void *opaque, hwaddr 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);
> +        sdbus_write_data(&s->sdbus, (value >> 24) & 0xff);
> +        sdbus_write_data(&s->sdbus, (value >> 16) & 0xff);
> +        sdbus_write_data(&s->sdbus, (value >> 8) & 0xff);
> +        sdbus_write_data(&s->sdbus, value & 0xff);
>          break;
>      case R_ENABLE:
>          s->regs[addr] = value;
> @@ -266,15 +266,23 @@ static int milkymist_memcard_init(SysBusDevice *dev)
>  static void milkymist_memcard_realize(DeviceState *dev, Error **errp)
>  {
>      MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
> +    DeviceState *carddev;
>      BlockBackend *blk;
>      DriveInfo *dinfo;
> +    Error *err = NULL;
>
> +    qbus_create_inplace(&s->sdbus, sizeof(s->sdbus), TYPE_SD_BUS,
> +                        dev, "sd-bus");
> +
> +    /* Create and plug in the sd card */
>      /* FIXME use a qdev drive property instead of drive_get_next() */
>      dinfo = drive_get_next(IF_SD);
>      blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
> -    s->card = sd_init(blk, false);
> -    if (s->card == NULL) {
> -        error_setg(errp, "failed to init SD card");
> +    carddev = qdev_create(&s->sdbus.qbus, TYPE_SD_CARD);
> +    qdev_prop_set_drive(carddev, "drive", blk, &err);
> +    object_property_set_bool(OBJECT(carddev), true, "realized", &err);
> +    if (err) {
> +        error_setg(errp, "failed to init SD card: %s", error_get_pretty(err));
>          return;
>      }
>      s->enabled = blk && blk_is_inserted(blk);
> --
> 2.15.1
>
>

Re: [Qemu-devel] [PATCH 4/6] hw/sd/pl181: expose a SDBus and connect the SDCard to it

[Alistair Francis]

On Wed, Jan 3, 2018 at 8:23 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> using the sdbus_*() API.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/pl181.c | 27 ++++++++++++++++++++-------
>  1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
> index 55c8098ecd..ce696c5d7d 100644
> --- a/hw/sd/pl181.c
> +++ b/hw/sd/pl181.c
> @@ -33,6 +33,7 @@ typedef struct PL181State {
>      SysBusDevice parent_obj;
>
>      MemoryRegion iomem;
> +    SDBus sdbus;
>      SDState *card;
>      uint32_t clock;
>      uint32_t power;
> @@ -179,7 +180,7 @@ static void pl181_send_command(PL181State *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 = sdbus_do_command(&s->sdbus, &request, response);
>      if (rlen < 0)
>          goto error;
>      if (s->cmd & PL181_CMD_RESPONSE) {
> @@ -223,12 +224,12 @@ static void pl181_fifo_run(PL181State *s)
>      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 || sdbus_data_ready(&s->sdbus))
>              && !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)sdbus_read_data(&s->sdbus) << (n * 8);
>                  s->datacnt--;
>                  n++;
>                  if (n == 4) {
> @@ -249,7 +250,7 @@ static void pl181_fifo_run(PL181State *s)
>                  }
>                  n--;
>                  s->datacnt--;
> -                sd_write_data(s->card, value & 0xff);
> +                sdbus_write_data(&s->sdbus, value & 0xff);
>                  value >>= 8;
>              }
>          }
> @@ -498,14 +499,26 @@ static void pl181_init(Object *obj)
>  static void pl181_realize(DeviceState *dev, Error **errp)
>  {
>      PL181State *s = PL181(dev);
> +    DeviceState *carddev;
>      DriveInfo *dinfo;
> +    Error *err = NULL;
>
> +    qbus_create_inplace(&s->sdbus, sizeof(s->sdbus), TYPE_SD_BUS,
> +                        dev, "sd-bus");
> +
> +    /* Create and plug in the sd card */
>      /* FIXME use a qdev drive property instead of drive_get_next() */
>      dinfo = drive_get_next(IF_SD);
> -    s->card = sd_init(dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, false);
> -    if (s->card == NULL) {
> -        error_setg(errp, "sd_init failed");
> +    carddev = qdev_create(&s->sdbus.qbus, TYPE_SD_CARD);
> +    if (dinfo) {
> +        qdev_prop_set_drive(carddev, "drive", blk_by_legacy_dinfo(dinfo), &err);
> +    }
> +    object_property_set_bool(OBJECT(carddev), true, "realized", &err);
> +    if (err) {
> +        error_setg(errp, "failed to init SD card: %s", error_get_pretty(err));
> +        return;
>      }
> +    s->card = SD_CARD(carddev);
>  }
>
>  static void pl181_class_init(ObjectClass *klass, void *data)
> --
> 2.15.1
>
>

Re: [Qemu-devel] [PATCH 5/6] hw/sd/ssi-sd: expose a SDBus and connect the SDCard to it

[Alistair Francis]

On Wed, Jan 3, 2018 at 8:23 AM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> using the sdbus_*() API.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/ssi-sd.c | 27 +++++++++++++++++++--------
>  1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
> index 24001dc3e6..c8b27add84 100644
> --- a/hw/sd/ssi-sd.c
> +++ b/hw/sd/ssi-sd.c
> @@ -47,7 +47,7 @@ typedef struct {
>      int32_t arglen;
>      int32_t response_pos;
>      int32_t stopping;
> -    SDState *sd;
> +    SDBus sdbus;
>  } ssi_sd_state;
>
>  /* State word bits.  */
> @@ -97,7 +97,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 = sdbus_do_command(&s->sdbus, &request, longresp);
>              if (s->arglen <= 0) {
>                  s->arglen = 1;
>                  s->response[0] = 4;
> @@ -174,7 +174,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 (sdbus_data_ready(&s->sdbus)) {
>              DPRINTF("Data read\n");
>              s->mode = SSI_SD_DATA_START;
>          } else {
> @@ -187,8 +187,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 = sdbus_read_data(&s->sdbus);
> +        if (!sdbus_data_ready(&s->sdbus)) {
>              DPRINTF("Data read end\n");
>              s->mode = SSI_SD_CMD;
>          }
> @@ -239,14 +239,25 @@ static const VMStateDescription vmstate_ssi_sd = {
>  static void ssi_sd_realize(SSISlave *d, Error **errp)
>  {
>      ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, d);
> +    DeviceState *carddev;
>      DriveInfo *dinfo;
> +    Error *err = NULL;
>
>      s->mode = SSI_SD_CMD;
> +    qbus_create_inplace(&s->sdbus, sizeof(s->sdbus), TYPE_SD_BUS,
> +                        DEVICE(d), "sd-bus");
> +
> +    /* Create and plug in the sd card */
>      /* FIXME use a qdev drive property instead of drive_get_next() */
>      dinfo = drive_get_next(IF_SD);
> -    s->sd = sd_init(dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, true);
> -    if (s->sd == NULL) {
> -        error_setg(errp, "Device initialization failed.");
> +    carddev = qdev_create(&s->sdbus.qbus, TYPE_SD_CARD);
> +    if (dinfo) {
> +        qdev_prop_set_drive(carddev, "drive", blk_by_legacy_dinfo(dinfo), &err);
> +    }
> +    object_property_set_bool(OBJECT(carddev), true, "spi", &err);
> +    object_property_set_bool(OBJECT(carddev), true, "realized", &err);
> +    if (err) {
> +        error_setg(errp, "failed to init SD card: %s", error_get_pretty(err));
>          return;
>      }
>  }
> --
> 2.15.1
>
>

Re: [Qemu-devel] [PATCH 2/6] hw/sd/milkymist-memcard: split realize() out of SysBusDevice init()

[Michael Walle]

Hey Philippe,

Am 2018-01-03 17:23, schrieb Philippe Mathieu-Daudé:
> Create the SDCard in the realize() function.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/sd/milkymist-memcard.c | 24 +++++++++++++++---------
>  1 file changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
> index 5de3e00e2f..5df3a0f815 100644
> --- a/hw/sd/milkymist-memcard.c
> +++ b/hw/sd/milkymist-memcard.c
> @@ -255,24 +255,29 @@ static void milkymist_memcard_reset(DeviceState 
> *d)
>  static int milkymist_memcard_init(SysBusDevice *dev)
>  {
>      MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
> -    DriveInfo *dinfo;
> +
> +    memory_region_init_io(&s->regs_region, OBJECT(s), 
> &memcard_mmio_ops, s,
> +            "milkymist-memcard", R_MAX * 4);
> +    sysbus_init_mmio(dev, &s->regs_region);
> +
> +    return 0;
> +}

Creating the device (milkymist_memcard_create()) fails with an 
assertion:
   sysbus_mmio_map_common: Assertion `n >= 0 && n < dev->num_mmio' 
failed.

because dev->num_mmio is still 0. Seems that sysbus_init_mmio() hasn't 
been called yet. Do we have to move the milkymist_memcard_init() to 
.instance_init?

Eg.

diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 5df3a0f815..21c0e91926 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -252,15 +252,14 @@ static void milkymist_memcard_reset(DeviceState 
*d)
      }
  }

-static int milkymist_memcard_init(SysBusDevice *dev)
+static void milkymist_memcard_init(Object *obj)
  {
-    MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
+    MilkymistMemcardState *s = MILKYMIST_MEMCARD(obj);
+    SysBusDevice *dev = SYS_BUS_DEVICE(obj);

      memory_region_init_io(&s->regs_region, OBJECT(s), 
&memcard_mmio_ops, s,
              "milkymist-memcard", R_MAX * 4);
      sysbus_init_mmio(dev, &s->regs_region);
-
-    return 0;
  }

  static void milkymist_memcard_realize(DeviceState *dev, Error **errp)
@@ -300,9 +299,7 @@ static const VMStateDescription 
vmstate_milkymist_memcard = {
  static void milkymist_memcard_class_init(ObjectClass *klass, void 
*data)
  {
      DeviceClass *dc = DEVICE_CLASS(klass);
-    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);

-    k->init = milkymist_memcard_init;
      dc->realize = milkymist_memcard_realize;
      dc->reset = milkymist_memcard_reset;
      dc->vmsd = &vmstate_milkymist_memcard;
@@ -314,6 +311,7 @@ static const TypeInfo milkymist_memcard_info = {
      .name          = TYPE_MILKYMIST_MEMCARD,
      .parent        = TYPE_SYS_BUS_DEVICE,
      .instance_size = sizeof(MilkymistMemcardState),
+    .instance_init = milkymist_memcard_init,
      .class_init    = milkymist_memcard_class_init,
  };

-michael

> +
> +static void milkymist_memcard_realize(DeviceState *dev, Error **errp)
> +{
> +    MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
>      BlockBackend *blk;
> +    DriveInfo *dinfo;
> 
>      /* FIXME use a qdev drive property instead of drive_get_next() */
>      dinfo = drive_get_next(IF_SD);
>      blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
>      s->card = sd_init(blk, false);
>      if (s->card == NULL) {
> -        return -1;
> +        error_setg(errp, "failed to init SD card");
> +        return;
>      }
> -
>      s->enabled = blk && blk_is_inserted(blk);
> -
> -    memory_region_init_io(&s->regs_region, OBJECT(s), 
> &memcard_mmio_ops, s,
> -            "milkymist-memcard", R_MAX * 4);
> -    sysbus_init_mmio(dev, &s->regs_region);
> -
> -    return 0;
>  }
> 
>  static const VMStateDescription vmstate_milkymist_memcard = {
> @@ -298,6 +303,7 @@ static void
> milkymist_memcard_class_init(ObjectClass *klass, void *data)
>      SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
> 
>      k->init = milkymist_memcard_init;
> +    dc->realize = milkymist_memcard_realize;
>      dc->reset = milkymist_memcard_reset;
>      dc->vmsd = &vmstate_milkymist_memcard;
>      /* Reason: init() method uses drive_get_next() */

Re: [Qemu-devel] [PATCH 2/6] hw/sd/milkymist-memcard: split realize() out of SysBusDevice init()

[Philippe Mathieu-Daudé]

Hi Michael,

> Am 2018-01-03 17:23, schrieb Philippe Mathieu-Daudé:
>>
>> Create the SDCard in the realize() function.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  hw/sd/milkymist-memcard.c | 24 +++++++++++++++---------
>>  1 file changed, 15 insertions(+), 9 deletions(-)
>>
>> diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
>> index 5de3e00e2f..5df3a0f815 100644
>> --- a/hw/sd/milkymist-memcard.c
>> +++ b/hw/sd/milkymist-memcard.c
>> @@ -255,24 +255,29 @@ static void milkymist_memcard_reset(DeviceState *d)
>>  static int milkymist_memcard_init(SysBusDevice *dev)
>>  {
>>      MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
>> -    DriveInfo *dinfo;
>> +
>> +    memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops,
>> s,
>> +            "milkymist-memcard", R_MAX * 4);
>> +    sysbus_init_mmio(dev, &s->regs_region);
>> +
>> +    return 0;
>> +}
>
>
> Creating the device (milkymist_memcard_create()) fails with an assertion:
>   sysbus_mmio_map_common: Assertion `n >= 0 && n < dev->num_mmio' failed.

Thanks for trying this.

Odd 'make check-qtest-lm32' didn't catch this...

> because dev->num_mmio is still 0. Seems that sysbus_init_mmio() hasn't been
> called yet. Do we have to move the milkymist_memcard_init() to
> .instance_init?

Your patch seems correct :)

Do you have some milky one prebuilt image that I can use for testing?

Regards,

Phil.

>
> Eg.
>
> diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
> index 5df3a0f815..21c0e91926 100644
> --- a/hw/sd/milkymist-memcard.c
> +++ b/hw/sd/milkymist-memcard.c
> @@ -252,15 +252,14 @@ static void milkymist_memcard_reset(DeviceState *d)
>      }
>  }
>
> -static int milkymist_memcard_init(SysBusDevice *dev)
> +static void milkymist_memcard_init(Object *obj)
>  {
> -    MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
> +    MilkymistMemcardState *s = MILKYMIST_MEMCARD(obj);
> +    SysBusDevice *dev = SYS_BUS_DEVICE(obj);
>
>      memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops, s,
>              "milkymist-memcard", R_MAX * 4);
>      sysbus_init_mmio(dev, &s->regs_region);
> -
> -    return 0;
>  }
>
>  static void milkymist_memcard_realize(DeviceState *dev, Error **errp)
> @@ -300,9 +299,7 @@ static const VMStateDescription
> vmstate_milkymist_memcard = {
>  static void milkymist_memcard_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> -    SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>
> -    k->init = milkymist_memcard_init;
>      dc->realize = milkymist_memcard_realize;
>      dc->reset = milkymist_memcard_reset;
>      dc->vmsd = &vmstate_milkymist_memcard;
> @@ -314,6 +311,7 @@ static const TypeInfo milkymist_memcard_info = {
>      .name          = TYPE_MILKYMIST_MEMCARD,
>      .parent        = TYPE_SYS_BUS_DEVICE,
>      .instance_size = sizeof(MilkymistMemcardState),
> +    .instance_init = milkymist_memcard_init,
>      .class_init    = milkymist_memcard_class_init,
>  };
>
> -michael
>
>
>> +
>> +static void milkymist_memcard_realize(DeviceState *dev, Error **errp)
>> +{
>> +    MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
>>      BlockBackend *blk;
>> +    DriveInfo *dinfo;
>>
>>      /* FIXME use a qdev drive property instead of drive_get_next() */
>>      dinfo = drive_get_next(IF_SD);
>>      blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
>>      s->card = sd_init(blk, false);
>>      if (s->card == NULL) {
>> -        return -1;
>> +        error_setg(errp, "failed to init SD card");
>> +        return;
>>      }
>> -
>>      s->enabled = blk && blk_is_inserted(blk);
>> -
>> -    memory_region_init_io(&s->regs_region, OBJECT(s), &memcard_mmio_ops,
>> s,
>> -            "milkymist-memcard", R_MAX * 4);
>> -    sysbus_init_mmio(dev, &s->regs_region);
>> -
>> -    return 0;
>>  }
>>
>>  static const VMStateDescription vmstate_milkymist_memcard = {
>> @@ -298,6 +303,7 @@ static void
>> milkymist_memcard_class_init(ObjectClass *klass, void *data)
>>      SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
>>
>>      k->init = milkymist_memcard_init;
>> +    dc->realize = milkymist_memcard_realize;
>>      dc->reset = milkymist_memcard_reset;
>>      dc->vmsd = &vmstate_milkymist_memcard;
>>      /* Reason: init() method uses drive_get_next() */

Re: [Qemu-devel] [PATCH 2/6] hw/sd/milkymist-memcard: split realize() out of SysBusDevice init()

[Philippe Mathieu-Daudé]

>> Creating the device (milkymist_memcard_create()) fails with an assertion:
>>   sysbus_mmio_map_common: Assertion `n >= 0 && n < dev->num_mmio' failed.
>
> Thanks for trying this.
>
> Odd 'make check-qtest-lm32' didn't catch this...

Oh it actually does... My bad!

  GTESTER check-qtest-lm32
qemu-system-lm32: hw/core/sysbus.c:131: sysbus_mmio_map_common:
Assertion `n >= 0 && n < dev->num_mmio' failed.
Broken pipe
tests/Makefile.include:854: recipe for target 'check-qtest-lm32' failed
make: *** [check-qtest-lm32] Error 1

Re: [Qemu-devel] [PATCH 2/6] hw/sd/milkymist-memcard: split realize() out of SysBusDevice init()

[Michael Walle]

Hi Philippe,

Am 2018-01-09 19:16, schrieb Philippe Mathieu-Daudé:
> Hi Michael,
> 
>> Am 2018-01-03 17:23, schrieb Philippe Mathieu-Daudé:
>>> 
>>> Create the SDCard in the realize() function.
>>> 
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>  hw/sd/milkymist-memcard.c | 24 +++++++++++++++---------
>>>  1 file changed, 15 insertions(+), 9 deletions(-)
>>> 
>>> diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
>>> index 5de3e00e2f..5df3a0f815 100644
>>> --- a/hw/sd/milkymist-memcard.c
>>> +++ b/hw/sd/milkymist-memcard.c
>>> @@ -255,24 +255,29 @@ static void milkymist_memcard_reset(DeviceState 
>>> *d)
>>>  static int milkymist_memcard_init(SysBusDevice *dev)
>>>  {
>>>      MilkymistMemcardState *s = MILKYMIST_MEMCARD(dev);
>>> -    DriveInfo *dinfo;
>>> +
>>> +    memory_region_init_io(&s->regs_region, OBJECT(s), 
>>> &memcard_mmio_ops,
>>> s,
>>> +            "milkymist-memcard", R_MAX * 4);
>>> +    sysbus_init_mmio(dev, &s->regs_region);
>>> +
>>> +    return 0;
>>> +}
>> 
>> 
>> Creating the device (milkymist_memcard_create()) fails with an 
>> assertion:
>>   sysbus_mmio_map_common: Assertion `n >= 0 && n < dev->num_mmio' 
>> failed.
> 
> Thanks for trying this.
> 
> Odd 'make check-qtest-lm32' didn't catch this...
> 
>> because dev->num_mmio is still 0. Seems that sysbus_init_mmio() hasn't 
>> been
>> called yet. Do we have to move the milkymist_memcard_init() to
>> .instance_init?
> 
> Your patch seems correct :)
> 
> Do you have some milky one prebuilt image that I can use for testing?

You can use this one:
   http://milkymist.walle.cc/updates/2012-03-01/flickernoise

http://milkymist.walle.cc/README.qemu

-michael