qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci)
@ 2016-02-16 18:09 Peter Maydell
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 01/10] hw/sd/sdhci.c: Remove x-drive property Peter Maydell
                   ` (10 more replies)
  0 siblings, 11 replies; 16+ messages in thread
From: Peter Maydell @ 2016-02-16 18:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Alistair Francis, qemu-arm, Peter Crosthwaite

This series attempts to QOMify sd.c (the actual SD card model),
including a proper QOM bus between the controller and the card.

TLDR: just a rebase and fix of #includes so it builds on current
master. Still need review on patches 4, 5, 6, and 8.


This series removes the experimental x-drive property on sdhci-pci;
the syntax for using that device changes:

instead of using the x-drive property:

  -device sdhci-pci,x-drive=mydrive -drive id=mydrive,[...]

we create a specific sd device (which is autoplugged into the
sdhci-pci device's "sd-bus" bus if you don't manually connect them):

  -device sdhci-pci -drive id=mydrive,[...] -device sd-card,drive=mydrive


The basic structure of the patch series is:
 * QOMify sd.c itself
 * Add the QOM bus APIs
 * Convert sdhci to use the QOM bus APIs
 * QOMify pxa2xx_mmci
 * Convert pxa2xx_mmci to use the QOM bus APIs

Some notes on compatibility/bisection:
 * the old-style non-QOMified controllers (which get their drive
   via blk_by_legacy_dinfo() continue to work through the whole series
 * the only QOMified controller which doesn't do that is sdhci-pci,
   which uses the experimental x-drive property to take a drive. This
   support and property is removed in patch 1; support for the new
   syntax is added in patch 5.
   (Since we're breaking the old syntax anyway I chose to not try to
   introduce the new syntax in the same commit as breaking the old one;
   I think it makes the patches easier to review.)

I don't have any Xilinx test images, so I haven't been able to test
the changes to those boards (beyond confirming that the board doesn't
crash on startup and that 'info qtree' seems to show SD cards
connected to the right things). I have tested sdhci-pci and
the pxa2xx.

Changes v1->v2:
 * change from "sd"/TYPE_SD/SD() to "sd-card"/TYPE_SD_CARD/SD_CARD()
 * similarly SD_CLASS -> SD_CARD_CLASS; SD_GET_CLASS -> SD_CARD_GET_CLASS;
   SDClass -> SDCardClass
 * fix pxa cut-n-paste flub
 * use error_propagate() rather than assuming input errp is non-NULL
 * remove stray blank lines/etc
 * use the new QOM alias-this-bus functionality as SPI does for Xilinx
 * use ARRAY_SIZE rather than sizeof
 * fix failure to register vmstate
Changes v2->v3:
 * add a missing #include that meant we weren't building if applied
   to current master
 * made new file include qemu/osdep.h first

thanks
-- PMM

Peter Maydell (10):
  hw/sd/sdhci.c: Remove x-drive property
  hw/sd/sd.c: QOMify
  hw/sd/sd.c: Convert sd_reset() function into Device reset method
  hw/sd: Add QOM bus which SD cards plug in to
  hw/sd/sdhci.c: Update to use SDBus APIs
  sdhci_sysbus: Create SD card device in users, not the device itself
  hw/sd/pxa2xx_mmci: convert to SysBusDevice object
  hw/sd/pxa2xx_mmci: Update to use new SDBus APIs
  hw/sd/pxa2xx_mmci: Convert to VMStateDescription
  hw/sd/pxa2xx_mmci: Add reset function

 hw/arm/xilinx_zynq.c  |  17 ++-
 hw/arm/xlnx-ep108.c   |  21 ++++
 hw/arm/xlnx-zynqmp.c  |   8 ++
 hw/sd/Makefile.objs   |   2 +-
 hw/sd/core.c          | 146 ++++++++++++++++++++++++
 hw/sd/pxa2xx_mmci.c   | 306 ++++++++++++++++++++++++++++++++------------------
 hw/sd/sd.c            | 150 ++++++++++++++++++++-----
 hw/sd/sdhci.c         |  82 ++++++++------
 include/hw/sd/sd.h    |  65 +++++++++++
 include/hw/sd/sdhci.h |   3 +-
 10 files changed, 624 insertions(+), 176 deletions(-)
 create mode 100644 hw/sd/core.c

-- 
1.9.1

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

* [Qemu-devel] [PATCH v3 01/10] hw/sd/sdhci.c: Remove x-drive property
  2016-02-16 18:09 [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
@ 2016-02-16 18:09 ` Peter Maydell
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 02/10] hw/sd/sd.c: QOMify Peter Maydell
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2016-02-16 18:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Alistair Francis, qemu-arm, Peter Crosthwaite

The following commits will remove support for the old sdhci-pci
command line syntax using the x-drive property:
 -device sdhci-pci,x-drive=mydrive -drive id=mydrive,[...]
and replace it with an explicit sd device:
 -device sdhci-pci -drive id=mydrive,[...] -device sd,drive=mydrive

(This is OK because x-drive is experimental.)

This commit removes the x-drive property so that old style
command lines will fail with a reasonable error message:
  -device sdhci-pci,x-drive=mydrive: Property '.x-drive' not found

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 hw/sd/sdhci.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 30e3bf4..3d1eb85 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1220,12 +1220,6 @@ const VMStateDescription sdhci_vmstate = {
 /* Capabilities registers provide information on supported features of this
  * specific host controller implementation */
 static Property sdhci_pci_properties[] = {
-    /*
-     * We currently fuse controller and card into a single device
-     * model, but we intend to separate them.  For that purpose, the
-     * properties that belong to the card are marked as experimental.
-     */
-    DEFINE_PROP_DRIVE("x-drive", SDHCIState, blk),
     DEFINE_PROP_UINT32("capareg", SDHCIState, capareg,
             SDHC_CAPAB_REG_DEFAULT),
     DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
-- 
1.9.1

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

* [Qemu-devel] [PATCH v3 02/10] hw/sd/sd.c: QOMify
  2016-02-16 18:09 [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 01/10] hw/sd/sdhci.c: Remove x-drive property Peter Maydell
@ 2016-02-16 18:09 ` Peter Maydell
  2016-02-17 17:34   ` Alistair Francis
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 03/10] hw/sd/sd.c: Convert sd_reset() function into Device reset method Peter Maydell
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2016-02-16 18:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Alistair Francis, qemu-arm, Peter Crosthwaite

Turn the SD card into a QOM device.
This conversion only changes the device itself; the various
functions which are effectively methods on the device are not
touched at this point.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 hw/sd/sd.c         | 99 ++++++++++++++++++++++++++++++++++++++++++------------
 include/hw/sd/sd.h |  3 ++
 2 files changed, 80 insertions(+), 22 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index dd614b0..1681728 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -34,6 +34,8 @@
 #include "sysemu/block-backend.h"
 #include "hw/sd/sd.h"
 #include "qemu/bitmap.h"
+#include "hw/qdev-properties.h"
+#include "qemu/error-report.h"
 
 //#define DEBUG_SD 1
 
@@ -78,6 +80,8 @@ enum SDCardStates {
 };
 
 struct SDState {
+    DeviceState parent_obj;
+
     uint32_t mode;    /* current card mode, one of SDCardModes */
     int32_t state;    /* current card state, one of SDCardStates */
     uint32_t ocr;
@@ -473,34 +477,26 @@ static const VMStateDescription sd_vmstate = {
     }
 };
 
-/* 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
-   is asserted.  */
+/* Legacy initialization function for use by non-qdevified callers */
 SDState *sd_init(BlockBackend *blk, bool is_spi)
 {
-    SDState *sd;
+    DeviceState *dev;
+    Error *err = NULL;
 
-    if (blk && blk_is_read_only(blk)) {
-        fprintf(stderr, "sd_init: Cannot use read-only drive\n");
+    dev = qdev_create(NULL, TYPE_SD_CARD);
+    qdev_prop_set_drive(dev, "drive", blk, &err);
+    if (err) {
+        error_report("sd_init failed: %s", error_get_pretty(err));
         return NULL;
     }
-
-    sd = (SDState *) g_malloc0(sizeof(SDState));
-    sd->buf = blk_blockalign(blk, 512);
-    sd->spi = is_spi;
-    sd->enable = true;
-    sd->blk = blk;
-    sd_reset(sd);
-    if (sd->blk) {
-        /* Attach dev if not already attached.  (This call ignores an
-         * error return code if sd->blk is already attached.) */
-        /* FIXME ignoring blk_attach_dev() failure is dangerously brittle */
-        blk_attach_dev(sd->blk, sd);
-        blk_set_dev_ops(sd->blk, &sd_block_ops, sd);
+    qdev_prop_set_bit(dev, "spi", is_spi);
+    object_property_set_bool(OBJECT(dev), true, "realized", &err);
+    if (err) {
+        error_report("sd_init failed: %s", error_get_pretty(err));
+        return NULL;
     }
-    vmstate_register(NULL, -1, &sd_vmstate, sd);
-    return sd;
+
+    return SD_CARD(dev);
 }
 
 void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
@@ -1769,3 +1765,62 @@ void sd_enable(SDState *sd, bool enable)
 {
     sd->enable = enable;
 }
+
+static void sd_instance_init(Object *obj)
+{
+    SDState *sd = SD_CARD(obj);
+
+    sd->enable = true;
+}
+
+static void sd_realize(DeviceState *dev, Error **errp)
+{
+    SDState *sd = SD_CARD(dev);
+
+    if (sd->blk && blk_is_read_only(sd->blk)) {
+        error_setg(errp, "Cannot use read-only drive as SD card");
+        return;
+    }
+
+    sd->buf = blk_blockalign(sd->blk, 512);
+
+    if (sd->blk) {
+        blk_set_dev_ops(sd->blk, &sd_block_ops, sd);
+    }
+
+    sd_reset(sd);
+}
+
+static Property sd_properties[] = {
+    DEFINE_PROP_DRIVE("drive", SDState, blk),
+    /* 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
+     * is asserted.  */
+    DEFINE_PROP_BOOL("spi", SDState, spi, false),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void sd_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = sd_realize;
+    dc->props = sd_properties;
+    dc->vmsd = &sd_vmstate;
+}
+
+static const TypeInfo sd_info = {
+    .name = TYPE_SD_CARD,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(SDState),
+    .class_init = sd_class_init,
+    .instance_init = sd_instance_init,
+};
+
+static void sd_register_types(void)
+{
+    type_register_static(&sd_info);
+}
+
+type_init(sd_register_types)
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 79adb5b..404d589 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -68,6 +68,9 @@ typedef struct {
 
 typedef struct SDState SDState;
 
+#define TYPE_SD_CARD "sd-card"
+#define SD_CARD(obj) OBJECT_CHECK(SDState, (obj), TYPE_SD_CARD)
+
 SDState *sd_init(BlockBackend *bs, bool is_spi);
 int sd_do_command(SDState *sd, SDRequest *req,
                   uint8_t *response);
-- 
1.9.1

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

* [Qemu-devel] [PATCH v3 03/10] hw/sd/sd.c: Convert sd_reset() function into Device reset method
  2016-02-16 18:09 [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 01/10] hw/sd/sdhci.c: Remove x-drive property Peter Maydell
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 02/10] hw/sd/sd.c: QOMify Peter Maydell
@ 2016-02-16 18:09 ` Peter Maydell
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 04/10] hw/sd: Add QOM bus which SD cards plug in to Peter Maydell
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2016-02-16 18:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Alistair Francis, qemu-arm, Peter Crosthwaite

Convert the sd_reset() function into a proper Device reset method.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 hw/sd/sd.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 1681728..edd4c82 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -394,8 +394,9 @@ static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
     return addr >> (HWBLOCK_SHIFT + SECTOR_SHIFT + WPGROUP_SHIFT);
 }
 
-static void sd_reset(SDState *sd)
+static void sd_reset(DeviceState *dev)
 {
+    SDState *sd = SD_CARD(dev);
     uint64_t size;
     uint64_t sect;
 
@@ -436,7 +437,7 @@ static void sd_cardchange(void *opaque, bool load)
 
     qemu_set_irq(sd->inserted_cb, blk_is_inserted(sd->blk));
     if (blk_is_inserted(sd->blk)) {
-        sd_reset(sd);
+        sd_reset(DEVICE(sd));
         qemu_set_irq(sd->readonly_cb, sd->wp_switch);
     }
 }
@@ -680,7 +681,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 
         default:
             sd->state = sd_idle_state;
-            sd_reset(sd);
+            sd_reset(DEVICE(sd));
             return sd->spi ? sd_r1 : sd_r0;
         }
         break;
@@ -1787,8 +1788,6 @@ static void sd_realize(DeviceState *dev, Error **errp)
     if (sd->blk) {
         blk_set_dev_ops(sd->blk, &sd_block_ops, sd);
     }
-
-    sd_reset(sd);
 }
 
 static Property sd_properties[] = {
@@ -1808,6 +1807,7 @@ static void sd_class_init(ObjectClass *klass, void *data)
     dc->realize = sd_realize;
     dc->props = sd_properties;
     dc->vmsd = &sd_vmstate;
+    dc->reset = sd_reset;
 }
 
 static const TypeInfo sd_info = {
-- 
1.9.1

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

* [Qemu-devel] [PATCH v3 04/10] hw/sd: Add QOM bus which SD cards plug in to
  2016-02-16 18:09 [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (2 preceding siblings ...)
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 03/10] hw/sd/sd.c: Convert sd_reset() function into Device reset method Peter Maydell
@ 2016-02-16 18:09 ` Peter Maydell
  2016-02-17 17:34   ` Alistair Francis
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 05/10] hw/sd/sdhci.c: Update to use SDBus APIs Peter Maydell
                   ` (6 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2016-02-16 18:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Alistair Francis, qemu-arm, Peter Crosthwaite

Add a QOM bus for SD cards to plug in to.

Note that since sd_enable() is used only by one board and there
only as part of a broken implementation, we do not provide it in
the SDBus API (but instead add a warning comment about the old
function). Whoever converts OMAP and the nseries boards to QOM
will need to either implement the card switch properly or move
the enable hack into the OMAP MMC controller model.

In the SDBus API, the old-style use of sd_set_cb to register some
qemu_irqs for notification of card insertion and write-protect
toggling is replaced with methods in the SDBusClass which the
card calls on status changes and methods in the SDClass which
the controller can call to find out the current status. The
query methods will allow us to remove the abuse of the 'register
irqs' API by controllers in their reset methods to trigger
the card to tell them about the current status again.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sd/Makefile.objs |   2 +-
 hw/sd/core.c        | 146 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/sd/sd.c          |  47 +++++++++++++++--
 include/hw/sd/sd.h  |  62 ++++++++++++++++++++++
 4 files changed, 252 insertions(+), 5 deletions(-)
 create mode 100644 hw/sd/core.c

diff --git a/hw/sd/Makefile.objs b/hw/sd/Makefile.objs
index f1aed83..31c8330 100644
--- a/hw/sd/Makefile.objs
+++ b/hw/sd/Makefile.objs
@@ -1,6 +1,6 @@
 common-obj-$(CONFIG_PL181) += pl181.o
 common-obj-$(CONFIG_SSI_SD) += ssi-sd.o
-common-obj-$(CONFIG_SD) += sd.o
+common-obj-$(CONFIG_SD) += sd.o core.o
 common-obj-$(CONFIG_SDHCI) += sdhci.o
 
 obj-$(CONFIG_MILKYMIST) += milkymist-memcard.o
diff --git a/hw/sd/core.c b/hw/sd/core.c
new file mode 100644
index 0000000..14c2bdf
--- /dev/null
+++ b/hw/sd/core.c
@@ -0,0 +1,146 @@
+/*
+ * SD card bus interface code.
+ *
+ * Copyright (c) 2015 Linaro Limited
+ *
+ * Author:
+ *  Peter Maydell <peter.maydell@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev-core.h"
+#include "sysemu/block-backend.h"
+#include "hw/sd/sd.h"
+
+static SDState *get_card(SDBus *sdbus)
+{
+    /* We only ever have one child on the bus so just return it */
+    BusChild *kid = QTAILQ_FIRST(&sdbus->qbus.children);
+
+    if (!kid) {
+        return NULL;
+    }
+    return SD_CARD(kid->child);
+}
+
+int sdbus_do_command(SDBus *sdbus, SDRequest *req, uint8_t *response)
+{
+    SDState *card = get_card(sdbus);
+
+    if (card) {
+        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+
+        return sc->do_command(card, req, response);
+    }
+
+    return 0;
+}
+
+void sdbus_write_data(SDBus *sdbus, uint8_t value)
+{
+    SDState *card = get_card(sdbus);
+
+    if (card) {
+        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+
+        sc->write_data(card, value);
+    }
+}
+
+uint8_t sdbus_read_data(SDBus *sdbus)
+{
+    SDState *card = get_card(sdbus);
+
+    if (card) {
+        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+
+        return sc->read_data(card);
+    }
+
+    return 0;
+}
+
+bool sdbus_data_ready(SDBus *sdbus)
+{
+    SDState *card = get_card(sdbus);
+
+    if (card) {
+        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+
+        return sc->data_ready(card);
+    }
+
+    return false;
+}
+
+bool sdbus_get_inserted(SDBus *sdbus)
+{
+    SDState *card = get_card(sdbus);
+
+    if (card) {
+        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+
+        return sc->get_inserted(card);
+    }
+
+    return false;
+}
+
+bool sdbus_get_readonly(SDBus *sdbus)
+{
+    SDState *card = get_card(sdbus);
+
+    if (card) {
+        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+
+        return sc->get_readonly(card);
+    }
+
+    return false;
+}
+
+void sdbus_set_inserted(SDBus *sdbus, bool inserted)
+{
+    SDBusClass *sbc = SD_BUS_GET_CLASS(sdbus);
+    BusState *qbus = BUS(sdbus);
+
+    if (sbc->set_inserted) {
+        sbc->set_inserted(qbus->parent, inserted);
+    }
+}
+
+void sdbus_set_readonly(SDBus *sdbus, bool readonly)
+{
+    SDBusClass *sbc = SD_BUS_GET_CLASS(sdbus);
+    BusState *qbus = BUS(sdbus);
+
+    if (sbc->set_readonly) {
+        sbc->set_readonly(qbus->parent, readonly);
+    }
+}
+
+static const TypeInfo sd_bus_info = {
+    .name = TYPE_SD_BUS,
+    .parent = TYPE_BUS,
+    .instance_size = sizeof(SDBus),
+    .class_size = sizeof(SDBusClass),
+};
+
+static void sd_bus_register_types(void)
+{
+    type_register_static(&sd_bus_info);
+}
+
+type_init(sd_bus_register_types)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index edd4c82..8902cd8 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -30,6 +30,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "hw/qdev.h"
 #include "hw/hw.h"
 #include "sysemu/block-backend.h"
 #include "hw/sd/sd.h"
@@ -431,14 +432,41 @@ static void sd_reset(DeviceState *dev)
     sd->expecting_acmd = false;
 }
 
+static bool sd_get_inserted(SDState *sd)
+{
+    return blk_is_inserted(sd->blk);
+}
+
+static bool sd_get_readonly(SDState *sd)
+{
+    return sd->wp_switch;
+}
+
 static void sd_cardchange(void *opaque, bool load)
 {
     SDState *sd = opaque;
+    DeviceState *dev = DEVICE(sd);
+    SDBus *sdbus = SD_BUS(qdev_get_parent_bus(dev));
+    bool inserted = sd_get_inserted(sd);
+    bool readonly = sd_get_readonly(sd);
 
-    qemu_set_irq(sd->inserted_cb, blk_is_inserted(sd->blk));
-    if (blk_is_inserted(sd->blk)) {
-        sd_reset(DEVICE(sd));
-        qemu_set_irq(sd->readonly_cb, sd->wp_switch);
+    if (inserted) {
+        sd_reset(dev);
+    }
+
+    /* The IRQ notification is for legacy non-QOM SD controller devices;
+     * QOMified controllers use the SDBus APIs.
+     */
+    if (sdbus) {
+        sdbus_set_inserted(sdbus, inserted);
+        if (inserted) {
+            sdbus_set_readonly(sdbus, readonly);
+        }
+    } else {
+        qemu_set_irq(sd->inserted_cb, inserted);
+        if (inserted) {
+            qemu_set_irq(sd->readonly_cb, readonly);
+        }
     }
 }
 
@@ -1803,17 +1831,28 @@ static Property sd_properties[] = {
 static void sd_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
+    SDCardClass *sc = SD_CARD_CLASS(klass);
 
     dc->realize = sd_realize;
     dc->props = sd_properties;
     dc->vmsd = &sd_vmstate;
     dc->reset = sd_reset;
+    dc->bus_type = TYPE_SD_BUS;
+
+    sc->do_command = sd_do_command;
+    sc->write_data = sd_write_data;
+    sc->read_data = sd_read_data;
+    sc->data_ready = sd_data_ready;
+    sc->enable = sd_enable;
+    sc->get_inserted = sd_get_inserted;
+    sc->get_readonly = sd_get_readonly;
 }
 
 static const TypeInfo sd_info = {
     .name = TYPE_SD_CARD,
     .parent = TYPE_DEVICE,
     .instance_size = sizeof(SDState),
+    .class_size = sizeof(SDCardClass),
     .class_init = sd_class_init,
     .instance_init = sd_instance_init,
 };
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 404d589..d5d273a 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -67,10 +67,51 @@ typedef struct {
 } SDRequest;
 
 typedef struct SDState SDState;
+typedef struct SDBus SDBus;
 
 #define TYPE_SD_CARD "sd-card"
 #define SD_CARD(obj) OBJECT_CHECK(SDState, (obj), TYPE_SD_CARD)
+#define SD_CARD_CLASS(klass) \
+    OBJECT_CLASS_CHECK(SDCardClass, (klass), TYPE_SD_CARD)
+#define SD_CARD_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(SDCardClass, (obj), TYPE_SD_CARD)
 
+typedef struct {
+    /*< private >*/
+    DeviceClass parent_class;
+    /*< public >*/
+
+    int (*do_command)(SDState *sd, SDRequest *req, uint8_t *response);
+    void (*write_data)(SDState *sd, uint8_t value);
+    uint8_t (*read_data)(SDState *sd);
+    bool (*data_ready)(SDState *sd);
+    void (*enable)(SDState *sd, bool enable);
+    bool (*get_inserted)(SDState *sd);
+    bool (*get_readonly)(SDState *sd);
+} SDCardClass;
+
+#define TYPE_SD_BUS "sd-bus"
+#define SD_BUS(obj) OBJECT_CHECK(SDBus, (obj), TYPE_SD_BUS)
+#define SD_BUS_CLASS(klass) OBJECT_CLASS_CHECK(SDBusClass, (klass), TYPE_SD_BUS)
+#define SD_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(SDBusClass, (obj), TYPE_SD_BUS)
+
+struct SDBus {
+    BusState qbus;
+};
+
+typedef struct {
+    /*< private >*/
+    BusClass parent_class;
+    /*< public >*/
+
+    /* These methods are called by the SD device to notify the controller
+     * when the card insertion or readonly status changes
+     */
+    void (*set_inserted)(DeviceState *dev, bool inserted);
+    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);
@@ -78,6 +119,27 @@ 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)
+ */
+int sdbus_do_command(SDBus *sd, SDRequest *req, uint8_t *response);
+void sdbus_write_data(SDBus *sd, uint8_t value);
+uint8_t sdbus_read_data(SDBus *sd);
+bool sdbus_data_ready(SDBus *sd);
+bool sdbus_get_inserted(SDBus *sd);
+bool sdbus_get_readonly(SDBus *sd);
+
+/* Functions to be used by SD devices to report back to qdevified controllers */
+void sdbus_set_inserted(SDBus *sd, bool inserted);
+void sdbus_set_readonly(SDBus *sd, bool inserted);
+
 #endif	/* __hw_sd_h */
-- 
1.9.1

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

* [Qemu-devel] [PATCH v3 05/10] hw/sd/sdhci.c: Update to use SDBus APIs
  2016-02-16 18:09 [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (3 preceding siblings ...)
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 04/10] hw/sd: Add QOM bus which SD cards plug in to Peter Maydell
@ 2016-02-16 18:09 ` Peter Maydell
  2016-02-17 18:01   ` Alistair Francis
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 06/10] sdhci_sysbus: Create SD card device in users, not the device itself Peter Maydell
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2016-02-16 18:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Alistair Francis, qemu-arm, Peter Crosthwaite

Update the SDHCI code to use the new SDBus APIs.

This commit introduces the new command line options required
to connect a disk to sdhci-pci:

 -device sdhci-pci -drive id=mydrive,[...] -device sd,drive=mydrive

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sd/sdhci.c         | 97 ++++++++++++++++++++++++++++++++++++---------------
 include/hw/sd/sdhci.h |  3 +-
 2 files changed, 69 insertions(+), 31 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 3d1eb85..396dd10 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -55,6 +55,9 @@
         } \
     } while (0)
 
+#define TYPE_SDHCI_BUS "sdhci-bus"
+#define SDHCI_BUS(obj) OBJECT_CHECK(SDBus, (obj), TYPE_SDHCI_BUS)
+
 /* Default SD/MMC host controller features information, which will be
  * presented in CAPABILITIES register of generic SD host controller at reset.
  * If not stated otherwise:
@@ -145,9 +148,9 @@ static void sdhci_raise_insertion_irq(void *opaque)
     }
 }
 
-static void sdhci_insert_eject_cb(void *opaque, int irq, int level)
+static void sdhci_set_inserted(DeviceState *dev, bool level)
 {
-    SDHCIState *s = (SDHCIState *)opaque;
+    SDHCIState *s = (SDHCIState *)dev;
     DPRINT_L1("Card state changed: %s!\n", level ? "insert" : "eject");
 
     if ((s->norintsts & SDHC_NIS_REMOVE) && level) {
@@ -172,9 +175,9 @@ static void sdhci_insert_eject_cb(void *opaque, int irq, int level)
     }
 }
 
-static void sdhci_card_readonly_cb(void *opaque, int irq, int level)
+static void sdhci_set_readonly(DeviceState *dev, bool level)
 {
-    SDHCIState *s = (SDHCIState *)opaque;
+    SDHCIState *s = (SDHCIState *)dev;
 
     if (level) {
         s->prnsts &= ~SDHC_WRITE_PROTECT;
@@ -186,6 +189,8 @@ static void sdhci_card_readonly_cb(void *opaque, int irq, int level)
 
 static void sdhci_reset(SDHCIState *s)
 {
+    DeviceState *dev = DEVICE(s);
+
     timer_del(s->insert_timer);
     timer_del(s->transfer_timer);
     /* Set all registers to 0. Capabilities registers are not cleared
@@ -194,8 +199,11 @@ static void sdhci_reset(SDHCIState *s)
     memset(&s->sdmasysad, 0, (uintptr_t)&s->capareg - (uintptr_t)&s->sdmasysad);
 
     if (!s->noeject_quirk) {
-        sd_set_cb(s->card, s->ro_cb, s->eject_cb);
+        /* Reset other state based on current card insertion/readonly status */
+        sdhci_set_inserted(dev, sdbus_get_inserted(&s->sdbus));
+        sdhci_set_readonly(dev, sdbus_get_readonly(&s->sdbus));
     }
+
     s->data_count = 0;
     s->stopped_state = sdhc_not_stopped;
 }
@@ -213,7 +221,7 @@ static void sdhci_send_command(SDHCIState *s)
     request.cmd = s->cmdreg >> 8;
     request.arg = s->argument;
     DPRINT_L1("sending CMD%u ARG[0x%08x]\n", request.cmd, request.arg);
-    rlen = sd_do_command(s->card, &request, response);
+    rlen = sdbus_do_command(&s->sdbus, &request, response);
 
     if (s->cmdreg & SDHC_CMD_RESPONSE) {
         if (rlen == 4) {
@@ -269,7 +277,7 @@ static void sdhci_end_transfer(SDHCIState *s)
         request.cmd = 0x0C;
         request.arg = 0;
         DPRINT_L1("Automatically issue CMD%d %08x\n", request.cmd, request.arg);
-        sd_do_command(s->card, &request, response);
+        sdbus_do_command(&s->sdbus, &request, response);
         /* Auto CMD12 response goes to the upper Response register */
         s->rspreg[3] = (response[0] << 24) | (response[1] << 16) |
                 (response[2] << 8) | response[3];
@@ -301,7 +309,7 @@ static void sdhci_read_block_from_card(SDHCIState *s)
     }
 
     for (index = 0; index < (s->blksize & 0x0fff); index++) {
-        s->fifo_buffer[index] = sd_read_data(s->card);
+        s->fifo_buffer[index] = sdbus_read_data(&s->sdbus);
     }
 
     /* New data now available for READ through Buffer Port Register */
@@ -394,7 +402,7 @@ static void sdhci_write_block_to_card(SDHCIState *s)
     }
 
     for (index = 0; index < (s->blksize & 0x0fff); index++) {
-        sd_write_data(s->card, s->fifo_buffer[index]);
+        sdbus_write_data(&s->sdbus, s->fifo_buffer[index]);
     }
 
     /* Next data can be written through BUFFER DATORT register */
@@ -476,7 +484,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
         while (s->blkcnt) {
             if (s->data_count == 0) {
                 for (n = 0; n < block_size; n++) {
-                    s->fifo_buffer[n] = sd_read_data(s->card);
+                    s->fifo_buffer[n] = sdbus_read_data(&s->sdbus);
                 }
             }
             begin = s->data_count;
@@ -517,7 +525,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
             s->sdmasysad += s->data_count - begin;
             if (s->data_count == block_size) {
                 for (n = 0; n < block_size; n++) {
-                    sd_write_data(s->card, s->fifo_buffer[n]);
+                    sdbus_write_data(&s->sdbus, s->fifo_buffer[n]);
                 }
                 s->data_count = 0;
                 if (s->trnmod & SDHC_TRNS_BLK_CNT_EN) {
@@ -549,7 +557,7 @@ static void sdhci_sdma_transfer_single_block(SDHCIState *s)
 
     if (s->trnmod & SDHC_TRNS_READ) {
         for (n = 0; n < datacnt; n++) {
-            s->fifo_buffer[n] = sd_read_data(s->card);
+            s->fifo_buffer[n] = sdbus_read_data(&s->sdbus);
         }
         dma_memory_write(&address_space_memory, s->sdmasysad, s->fifo_buffer,
                          datacnt);
@@ -557,7 +565,7 @@ static void sdhci_sdma_transfer_single_block(SDHCIState *s)
         dma_memory_read(&address_space_memory, s->sdmasysad, s->fifo_buffer,
                         datacnt);
         for (n = 0; n < datacnt; n++) {
-            sd_write_data(s->card, s->fifo_buffer[n]);
+            sdbus_write_data(&s->sdbus, s->fifo_buffer[n]);
         }
     }
 
@@ -661,7 +669,7 @@ static void sdhci_do_adma(SDHCIState *s)
                 while (length) {
                     if (s->data_count == 0) {
                         for (n = 0; n < block_size; n++) {
-                            s->fifo_buffer[n] = sd_read_data(s->card);
+                            s->fifo_buffer[n] = sdbus_read_data(&s->sdbus);
                         }
                     }
                     begin = s->data_count;
@@ -702,7 +710,7 @@ static void sdhci_do_adma(SDHCIState *s)
                     dscr.addr += s->data_count - begin;
                     if (s->data_count == block_size) {
                         for (n = 0; n < block_size; n++) {
-                            sd_write_data(s->card, s->fifo_buffer[n]);
+                            sdbus_write_data(&s->sdbus, s->fifo_buffer[n]);
                         }
                         s->data_count = 0;
                         if (s->trnmod & SDHC_TRNS_BLK_CNT_EN) {
@@ -816,7 +824,7 @@ static void sdhci_data_transfer(void *opaque)
             break;
         }
     } else {
-        if ((s->trnmod & SDHC_TRNS_READ) && sd_data_ready(s->card)) {
+        if ((s->trnmod & SDHC_TRNS_READ) && sdbus_data_ready(&s->sdbus)) {
             s->prnsts |= SDHC_DOING_READ | SDHC_DATA_INHIBIT |
                     SDHC_DAT_LINE_ACTIVE;
             sdhci_read_block_from_card(s);
@@ -1153,15 +1161,10 @@ static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
     }
 }
 
-static void sdhci_initfn(SDHCIState *s, BlockBackend *blk)
+static void sdhci_initfn(SDHCIState *s)
 {
-    s->card = sd_init(blk, false);
-    if (s->card == NULL) {
-        exit(1);
-    }
-    s->eject_cb = qemu_allocate_irq(sdhci_insert_eject_cb, s, 0);
-    s->ro_cb = qemu_allocate_irq(sdhci_card_readonly_cb, s, 0);
-    sd_set_cb(s->card, s->ro_cb, s->eject_cb);
+    qbus_create_inplace(&s->sdbus, sizeof(s->sdbus),
+                        TYPE_SDHCI_BUS, DEVICE(s), "sd-bus");
 
     s->insert_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_raise_insertion_irq, s);
     s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_data_transfer, s);
@@ -1231,7 +1234,7 @@ static void sdhci_pci_realize(PCIDevice *dev, Error **errp)
     SDHCIState *s = PCI_SDHCI(dev);
     dev->config[PCI_CLASS_PROG] = 0x01; /* Standard Host supported DMA */
     dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
-    sdhci_initfn(s, s->blk);
+    sdhci_initfn(s);
     s->buf_maxsz = sdhci_get_fifolen(s);
     s->fifo_buffer = g_malloc0(s->buf_maxsz);
     s->irq = pci_allocate_irq(dev);
@@ -1279,11 +1282,8 @@ static Property sdhci_sysbus_properties[] = {
 static void sdhci_sysbus_init(Object *obj)
 {
     SDHCIState *s = SYSBUS_SDHCI(obj);
-    DriveInfo *di;
 
-    /* FIXME use a qdev drive property instead of drive_get_next() */
-    di = drive_get_next(IF_SD);
-    sdhci_initfn(s, di ? blk_by_legacy_dinfo(di) : NULL);
+    sdhci_initfn(s);
 }
 
 static void sdhci_sysbus_finalize(Object *obj)
@@ -1296,6 +1296,29 @@ static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp)
 {
     SDHCIState *s = SYSBUS_SDHCI(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+    DriveInfo *di;
+    BlockBackend *blk;
+    DeviceState *carddev;
+    Error *err = NULL;
+
+    /* Create and plug in the sd card.
+     * FIXME: this should be done by the users of this device so we
+     * do not use drive_get_next() here.
+     */
+    di = drive_get_next(IF_SD);
+    blk = di ? blk_by_legacy_dinfo(di) : NULL;
+
+    carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
+    qdev_prop_set_drive(carddev, "drive", blk, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
+    object_property_set_bool(OBJECT(carddev), true, "realized", &err);
+    if (err) {
+        error_propagate(errp, err);
+        return;
+    }
 
     s->buf_maxsz = sdhci_get_fifolen(s);
     s->fifo_buffer = g_malloc0(s->buf_maxsz);
@@ -1325,10 +1348,26 @@ static const TypeInfo sdhci_sysbus_info = {
     .class_init = sdhci_sysbus_class_init,
 };
 
+static void sdhci_bus_class_init(ObjectClass *klass, void *data)
+{
+    SDBusClass *sbc = SD_BUS_CLASS(klass);
+
+    sbc->set_inserted = sdhci_set_inserted;
+    sbc->set_readonly = sdhci_set_readonly;
+}
+
+static const TypeInfo sdhci_bus_info = {
+    .name = TYPE_SDHCI_BUS,
+    .parent = TYPE_SD_BUS,
+    .instance_size = sizeof(SDBus),
+    .class_init = sdhci_bus_class_init,
+};
+
 static void sdhci_register_types(void)
 {
     type_register_static(&sdhci_pci_info);
     type_register_static(&sdhci_sysbus_info);
+    type_register_static(&sdhci_bus_info);
 }
 
 type_init(sdhci_register_types)
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index ffd1f80..607a83e 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -37,9 +37,8 @@ typedef struct SDHCIState {
         PCIDevice pcidev;
         SysBusDevice busdev;
     };
-    SDState *card;
+    SDBus sdbus;
     MemoryRegion iomem;
-    BlockBackend *blk;
 
     QEMUTimer *insert_timer;       /* timer for 'changing' sd card. */
     QEMUTimer *transfer_timer;
-- 
1.9.1

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

* [Qemu-devel] [PATCH v3 06/10] sdhci_sysbus: Create SD card device in users, not the device itself
  2016-02-16 18:09 [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (4 preceding siblings ...)
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 05/10] hw/sd/sdhci.c: Update to use SDBus APIs Peter Maydell
@ 2016-02-16 18:09 ` Peter Maydell
  2016-02-17 18:01   ` Alistair Francis
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 07/10] hw/sd/pxa2xx_mmci: convert to SysBusDevice object Peter Maydell
                   ` (4 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Peter Maydell @ 2016-02-16 18:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Alistair Francis, qemu-arm, Peter Crosthwaite

Move the creation of the SD card device from the sdhci_sysbus
device itself into the boards that create these devices.
This allows us to remove the cannot_instantiate_with_device_add
notation because we no longer call drive_get_next in the device
model.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/arm/xilinx_zynq.c | 17 ++++++++++++++++-
 hw/arm/xlnx-ep108.c  | 21 +++++++++++++++++++++
 hw/arm/xlnx-zynqmp.c |  8 ++++++++
 hw/sd/sdhci.c        | 25 -------------------------
 4 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 66e7f27..a35983a 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -28,6 +28,7 @@
 #include "hw/misc/zynq-xadc.h"
 #include "hw/ssi/ssi.h"
 #include "qemu/error-report.h"
+#include "hw/sd/sd.h"
 
 #define NUM_SPI_FLASHES 4
 #define NUM_QSPI_FLASHES 2
@@ -154,8 +155,10 @@ static void zynq_init(MachineState *machine)
     MemoryRegion *address_space_mem = get_system_memory();
     MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
     MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
-    DeviceState *dev;
+    DeviceState *dev, *carddev;
     SysBusDevice *busdev;
+    DriveInfo *di;
+    BlockBackend *blk;
     qemu_irq pic[64];
     int n;
 
@@ -245,11 +248,23 @@ static void zynq_init(MachineState *machine)
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0100000);
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[56-IRQ_OFFSET]);
 
+    di = drive_get_next(IF_SD);
+    blk = di ? blk_by_legacy_dinfo(di) : NULL;
+    carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
+    qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
+    object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal);
+
     dev = qdev_create(NULL, "generic-sdhci");
     qdev_init_nofail(dev);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000);
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[79-IRQ_OFFSET]);
 
+    di = drive_get_next(IF_SD);
+    blk = di ? blk_by_legacy_dinfo(di) : NULL;
+    carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
+    qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
+    object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal);
+
     dev = qdev_create(NULL, TYPE_ZYNQ_XADC);
     qdev_init_nofail(dev);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8007100);
diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
index 0de132a..a1bd283 100644
--- a/hw/arm/xlnx-ep108.c
+++ b/hw/arm/xlnx-ep108.c
@@ -59,6 +59,27 @@ static void xlnx_ep108_init(MachineState *machine)
 
     object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
 
+    /* Create and plug in the SD cards */
+    for (i = 0; i < XLNX_ZYNQMP_NUM_SDHCI; i++) {
+        BusState *bus;
+        DriveInfo *di = drive_get_next(IF_SD);
+        BlockBackend *blk = di ? blk_by_legacy_dinfo(di) : NULL;
+        DeviceState *carddev;
+        char *bus_name;
+
+        bus_name = g_strdup_printf("sd-bus%d", i);
+        bus = qdev_get_child_bus(DEVICE(&s->soc), bus_name);
+        g_free(bus_name);
+        if (!bus) {
+            error_report("No SD bus found for SD card %d", i);
+            exit(1);
+        }
+        carddev = qdev_create(bus, TYPE_SD_CARD);
+        qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
+        object_property_set_bool(OBJECT(carddev), true, "realized",
+                                 &error_fatal);
+    }
+
     for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
         SSIBus *spi_bus;
         DeviceState *flash_dev;
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 1508d08..4fbb635 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -327,6 +327,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->sata), 0, gic_spi[SATA_INTR]);
 
     for (i = 0; i < XLNX_ZYNQMP_NUM_SDHCI; i++) {
+        char *bus_name;
+
         object_property_set_bool(OBJECT(&s->sdhci[i]), true,
                                  "realized", &err);
         if (err) {
@@ -337,6 +339,12 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
                         sdhci_addr[i]);
         sysbus_connect_irq(SYS_BUS_DEVICE(&s->sdhci[i]), 0,
                            gic_spi[sdhci_intr[i]]);
+        /* Alias controller SD bus to the SoC itself */
+        bus_name = g_strdup_printf("sd-bus%d", i);
+        object_property_add_alias(OBJECT(s), bus_name,
+                                  OBJECT(&s->sdhci[i]), "sd-bus",
+                                  &error_abort);
+        g_free(bus_name);
     }
 
     for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 396dd10..73e7c87 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1296,29 +1296,6 @@ static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp)
 {
     SDHCIState *s = SYSBUS_SDHCI(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
-    DriveInfo *di;
-    BlockBackend *blk;
-    DeviceState *carddev;
-    Error *err = NULL;
-
-    /* Create and plug in the sd card.
-     * FIXME: this should be done by the users of this device so we
-     * do not use drive_get_next() here.
-     */
-    di = drive_get_next(IF_SD);
-    blk = di ? blk_by_legacy_dinfo(di) : NULL;
-
-    carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
-    qdev_prop_set_drive(carddev, "drive", blk, &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
-    object_property_set_bool(OBJECT(carddev), true, "realized", &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
 
     s->buf_maxsz = sdhci_get_fifolen(s);
     s->fifo_buffer = g_malloc0(s->buf_maxsz);
@@ -1335,8 +1312,6 @@ static void sdhci_sysbus_class_init(ObjectClass *klass, void *data)
     dc->vmsd = &sdhci_vmstate;
     dc->props = sdhci_sysbus_properties;
     dc->realize = sdhci_sysbus_realize;
-    /* Reason: instance_init() method uses drive_get_next() */
-    dc->cannot_instantiate_with_device_add_yet = true;
 }
 
 static const TypeInfo sdhci_sysbus_info = {
-- 
1.9.1

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

* [Qemu-devel] [PATCH v3 07/10] hw/sd/pxa2xx_mmci: convert to SysBusDevice object
  2016-02-16 18:09 [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (5 preceding siblings ...)
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 06/10] sdhci_sysbus: Create SD card device in users, not the device itself Peter Maydell
@ 2016-02-16 18:09 ` Peter Maydell
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 08/10] hw/sd/pxa2xx_mmci: Update to use new SDBus APIs Peter Maydell
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2016-02-16 18:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Alistair Francis, qemu-arm, Peter Crosthwaite

Convert the pxa2xx_mmci device to be a sysbus device.

In this commit we only change the device itself, and leave
the interface to the SD card using the old non-SDBus APIs.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 hw/sd/pxa2xx_mmci.c | 74 ++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 56 insertions(+), 18 deletions(-)

diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
index 81da7b7..cadbba3 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -12,16 +12,24 @@
 
 #include "qemu/osdep.h"
 #include "hw/hw.h"
+#include "hw/sysbus.h"
 #include "hw/arm/pxa.h"
 #include "hw/sd/sd.h"
 #include "hw/qdev.h"
+#include "hw/qdev-properties.h"
+
+#define TYPE_PXA2XX_MMCI "pxa2xx-mmci"
+#define PXA2XX_MMCI(obj) OBJECT_CHECK(PXA2xxMMCIState, (obj), TYPE_PXA2XX_MMCI)
+
+typedef struct PXA2xxMMCIState {
+    SysBusDevice parent_obj;
 
-struct PXA2xxMMCIState {
     MemoryRegion iomem;
     qemu_irq irq;
     qemu_irq rx_dma;
     qemu_irq tx_dma;
 
+    BlockBackend *blk;
     SDState *card;
 
     uint32_t status;
@@ -49,7 +57,7 @@ struct PXA2xxMMCIState {
     int resp_len;
 
     int cmdreq;
-};
+} PXA2xxMMCIState;
 
 #define MMC_STRPCL	0x00	/* MMC Clock Start/Stop register */
 #define MMC_STAT	0x04	/* MMC Status register */
@@ -475,31 +483,61 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
                 BlockBackend *blk, qemu_irq irq,
                 qemu_irq rx_dma, qemu_irq tx_dma)
 {
+    DeviceState *dev;
+    SysBusDevice *sbd;
     PXA2xxMMCIState *s;
 
-    s = (PXA2xxMMCIState *) g_malloc0(sizeof(PXA2xxMMCIState));
-    s->irq = irq;
-    s->rx_dma = rx_dma;
-    s->tx_dma = tx_dma;
-
-    memory_region_init_io(&s->iomem, NULL, &pxa2xx_mmci_ops, s,
-                          "pxa2xx-mmci", 0x00100000);
-    memory_region_add_subregion(sysmem, base, &s->iomem);
-
-    /* Instantiate the actual storage */
-    s->card = sd_init(blk, false);
+    dev = qdev_create(NULL, TYPE_PXA2XX_MMCI);
+    s = PXA2XX_MMCI(dev);
+    /* Reach into the device and initialize the SD card. This is
+     * unclean but will vanish when we update to SDBus APIs.
+     */
+    s->card = sd_init(s->blk, false);
     if (s->card == NULL) {
         exit(1);
     }
-
-    register_savevm(NULL, "pxa2xx_mmci", 0, 0,
-                    pxa2xx_mmci_save, pxa2xx_mmci_load, s);
-
+    qdev_init_nofail(dev);
+    sbd = SYS_BUS_DEVICE(dev);
+    sysbus_mmio_map(sbd, 0, base);
+    sysbus_connect_irq(sbd, 0, irq);
+    qdev_connect_gpio_out_named(dev, "rx-dma", 0, rx_dma);
+    qdev_connect_gpio_out_named(dev, "tx-dma", 0, tx_dma);
     return s;
 }
 
 void pxa2xx_mmci_handlers(PXA2xxMMCIState *s, qemu_irq readonly,
-                qemu_irq coverswitch)
+                          qemu_irq coverswitch)
 {
     sd_set_cb(s->card, readonly, coverswitch);
 }
+
+static void pxa2xx_mmci_instance_init(Object *obj)
+{
+    PXA2xxMMCIState *s = PXA2XX_MMCI(obj);
+    SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
+    DeviceState *dev = DEVICE(obj);
+
+    memory_region_init_io(&s->iomem, obj, &pxa2xx_mmci_ops, s,
+                          "pxa2xx-mmci", 0x00100000);
+    sysbus_init_mmio(sbd, &s->iomem);
+    sysbus_init_irq(sbd, &s->irq);
+    qdev_init_gpio_out_named(dev, &s->rx_dma, "rx-dma", 1);
+    qdev_init_gpio_out_named(dev, &s->tx_dma, "tx-dma", 1);
+
+    register_savevm(NULL, "pxa2xx_mmci", 0, 0,
+                    pxa2xx_mmci_save, pxa2xx_mmci_load, s);
+}
+
+static const TypeInfo pxa2xx_mmci_info = {
+    .name = TYPE_PXA2XX_MMCI,
+    .parent = TYPE_SYS_BUS_DEVICE,
+    .instance_size = sizeof(PXA2xxMMCIState),
+    .instance_init = pxa2xx_mmci_instance_init,
+};
+
+static void pxa2xx_mmci_register_types(void)
+{
+    type_register_static(&pxa2xx_mmci_info);
+}
+
+type_init(pxa2xx_mmci_register_types)
-- 
1.9.1

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

* [Qemu-devel] [PATCH v3 08/10] hw/sd/pxa2xx_mmci: Update to use new SDBus APIs
  2016-02-16 18:09 [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (6 preceding siblings ...)
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 07/10] hw/sd/pxa2xx_mmci: convert to SysBusDevice object Peter Maydell
@ 2016-02-16 18:09 ` Peter Maydell
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 09/10] hw/sd/pxa2xx_mmci: Convert to VMStateDescription Peter Maydell
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2016-02-16 18:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Alistair Francis, qemu-arm, Peter Crosthwaite

Now the PXA2xx MMCI device is QOMified itself, we can
update it to use the SDBus APIs to talk to the SD card.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sd/pxa2xx_mmci.c | 80 +++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 66 insertions(+), 14 deletions(-)

diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
index cadbba3..75986ea 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -17,10 +17,14 @@
 #include "hw/sd/sd.h"
 #include "hw/qdev.h"
 #include "hw/qdev-properties.h"
+#include "qemu/error-report.h"
 
 #define TYPE_PXA2XX_MMCI "pxa2xx-mmci"
 #define PXA2XX_MMCI(obj) OBJECT_CHECK(PXA2xxMMCIState, (obj), TYPE_PXA2XX_MMCI)
 
+#define TYPE_PXA2XX_MMCI_BUS "pxa2xx-mmci-bus"
+#define PXA2XX_MMCI_BUS(obj) OBJECT_CHECK(SDBus, (obj), TYPE_PXA2XX_MMCI_BUS)
+
 typedef struct PXA2xxMMCIState {
     SysBusDevice parent_obj;
 
@@ -28,9 +32,11 @@ typedef struct PXA2xxMMCIState {
     qemu_irq irq;
     qemu_irq rx_dma;
     qemu_irq tx_dma;
+    qemu_irq inserted;
+    qemu_irq readonly;
 
     BlockBackend *blk;
-    SDState *card;
+    SDBus sdbus;
 
     uint32_t status;
     uint32_t clkrt;
@@ -130,7 +136,7 @@ static void pxa2xx_mmci_fifo_update(PXA2xxMMCIState *s)
 
     if (s->cmdat & CMDAT_WR_RD) {
         while (s->bytesleft && s->tx_len) {
-            sd_write_data(s->card, s->tx_fifo[s->tx_start ++]);
+            sdbus_write_data(&s->sdbus, s->tx_fifo[s->tx_start++]);
             s->tx_start &= 0x1f;
             s->tx_len --;
             s->bytesleft --;
@@ -140,7 +146,7 @@ static void pxa2xx_mmci_fifo_update(PXA2xxMMCIState *s)
     } else
         while (s->bytesleft && s->rx_len < 32) {
             s->rx_fifo[(s->rx_start + (s->rx_len ++)) & 0x1f] =
-                sd_read_data(s->card);
+                sdbus_read_data(&s->sdbus);
             s->bytesleft --;
             s->intreq |= INT_RXFIFO_REQ;
         }
@@ -174,7 +180,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 = sdbus_do_command(&s->sdbus, &request, response);
     s->intreq |= INT_END_CMD;
 
     memset(s->resp_fifo, 0, sizeof(s->resp_fifo));
@@ -483,32 +489,59 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
                 BlockBackend *blk, qemu_irq irq,
                 qemu_irq rx_dma, qemu_irq tx_dma)
 {
-    DeviceState *dev;
+    DeviceState *dev, *carddev;
     SysBusDevice *sbd;
     PXA2xxMMCIState *s;
+    Error *err = NULL;
 
     dev = qdev_create(NULL, TYPE_PXA2XX_MMCI);
     s = PXA2XX_MMCI(dev);
-    /* Reach into the device and initialize the SD card. This is
-     * unclean but will vanish when we update to SDBus APIs.
-     */
-    s->card = sd_init(s->blk, false);
-    if (s->card == NULL) {
-        exit(1);
-    }
-    qdev_init_nofail(dev);
     sbd = SYS_BUS_DEVICE(dev);
     sysbus_mmio_map(sbd, 0, base);
     sysbus_connect_irq(sbd, 0, irq);
     qdev_connect_gpio_out_named(dev, "rx-dma", 0, rx_dma);
     qdev_connect_gpio_out_named(dev, "tx-dma", 0, tx_dma);
+
+    /* Create and plug in the sd card */
+    carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
+    qdev_prop_set_drive(carddev, "drive", blk, &err);
+    if (err) {
+        error_report("failed to init SD card: %s", error_get_pretty(err));
+        return NULL;
+    }
+    object_property_set_bool(OBJECT(carddev), true, "realized", &err);
+    if (err) {
+        error_report("failed to init SD card: %s", error_get_pretty(err));
+        return NULL;
+    }
+
     return s;
 }
 
+static void pxa2xx_mmci_set_inserted(DeviceState *dev, bool inserted)
+{
+    PXA2xxMMCIState *s = PXA2XX_MMCI(dev);
+
+    qemu_set_irq(s->inserted, inserted);
+}
+
+static void pxa2xx_mmci_set_readonly(DeviceState *dev, bool readonly)
+{
+    PXA2xxMMCIState *s = PXA2XX_MMCI(dev);
+
+    qemu_set_irq(s->readonly, readonly);
+}
+
 void pxa2xx_mmci_handlers(PXA2xxMMCIState *s, qemu_irq readonly,
                           qemu_irq coverswitch)
 {
-    sd_set_cb(s->card, readonly, coverswitch);
+    DeviceState *dev = DEVICE(s);
+
+    s->readonly = readonly;
+    s->inserted = coverswitch;
+
+    pxa2xx_mmci_set_inserted(dev, sdbus_get_inserted(&s->sdbus));
+    pxa2xx_mmci_set_readonly(dev, sdbus_get_readonly(&s->sdbus));
 }
 
 static void pxa2xx_mmci_instance_init(Object *obj)
@@ -526,6 +559,17 @@ static void pxa2xx_mmci_instance_init(Object *obj)
 
     register_savevm(NULL, "pxa2xx_mmci", 0, 0,
                     pxa2xx_mmci_save, pxa2xx_mmci_load, s);
+
+    qbus_create_inplace(&s->sdbus, sizeof(s->sdbus),
+                        TYPE_PXA2XX_MMCI_BUS, DEVICE(obj), "sd-bus");
+}
+
+static void pxa2xx_mmci_bus_class_init(ObjectClass *klass, void *data)
+{
+    SDBusClass *sbc = SD_BUS_CLASS(klass);
+
+    sbc->set_inserted = pxa2xx_mmci_set_inserted;
+    sbc->set_readonly = pxa2xx_mmci_set_readonly;
 }
 
 static const TypeInfo pxa2xx_mmci_info = {
@@ -535,9 +579,17 @@ static const TypeInfo pxa2xx_mmci_info = {
     .instance_init = pxa2xx_mmci_instance_init,
 };
 
+static const TypeInfo pxa2xx_mmci_bus_info = {
+    .name = TYPE_PXA2XX_MMCI_BUS,
+    .parent = TYPE_SD_BUS,
+    .instance_size = sizeof(SDBus),
+    .class_init = pxa2xx_mmci_bus_class_init,
+};
+
 static void pxa2xx_mmci_register_types(void)
 {
     type_register_static(&pxa2xx_mmci_info);
+    type_register_static(&pxa2xx_mmci_bus_info);
 }
 
 type_init(pxa2xx_mmci_register_types)
-- 
1.9.1

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

* [Qemu-devel] [PATCH v3 09/10] hw/sd/pxa2xx_mmci: Convert to VMStateDescription
  2016-02-16 18:09 [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (7 preceding siblings ...)
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 08/10] hw/sd/pxa2xx_mmci: Update to use new SDBus APIs Peter Maydell
@ 2016-02-16 18:09 ` Peter Maydell
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 10/10] hw/sd/pxa2xx_mmci: Add reset function Peter Maydell
  2016-02-18 11:48 ` [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
  10 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2016-02-16 18:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Alistair Francis, qemu-arm, Peter Crosthwaite

Convert the pxa2xx_mmci device from manual save/load
functions to a VMStateDescription structure.

This is a migration compatibility break.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 hw/sd/pxa2xx_mmci.c | 156 +++++++++++++++++++++-------------------------------
 1 file changed, 64 insertions(+), 92 deletions(-)

diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
index 75986ea..d9f5202 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -44,27 +44,72 @@ typedef struct PXA2xxMMCIState {
     uint32_t cmdat;
     uint32_t resp_tout;
     uint32_t read_tout;
-    int blklen;
-    int numblk;
+    int32_t blklen;
+    int32_t numblk;
     uint32_t intmask;
     uint32_t intreq;
-    int cmd;
+    int32_t cmd;
     uint32_t arg;
 
-    int active;
-    int bytesleft;
+    int32_t active;
+    int32_t bytesleft;
     uint8_t tx_fifo[64];
-    int tx_start;
-    int tx_len;
+    uint32_t tx_start;
+    uint32_t tx_len;
     uint8_t rx_fifo[32];
-    int rx_start;
-    int rx_len;
+    uint32_t rx_start;
+    uint32_t rx_len;
     uint16_t resp_fifo[9];
-    int resp_len;
+    uint32_t resp_len;
 
-    int cmdreq;
+    int32_t cmdreq;
 } PXA2xxMMCIState;
 
+static bool pxa2xx_mmci_vmstate_validate(void *opaque, int version_id)
+{
+    PXA2xxMMCIState *s = opaque;
+
+    return s->tx_start < ARRAY_SIZE(s->tx_fifo)
+        && s->rx_start < ARRAY_SIZE(s->rx_fifo)
+        && s->tx_len <= ARRAY_SIZE(s->tx_fifo)
+        && s->rx_len <= ARRAY_SIZE(s->rx_fifo)
+        && s->resp_len <= ARRAY_SIZE(s->resp_fifo);
+}
+
+
+static const VMStateDescription vmstate_pxa2xx_mmci = {
+    .name = "pxa2xx-mmci",
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(status, PXA2xxMMCIState),
+        VMSTATE_UINT32(clkrt, PXA2xxMMCIState),
+        VMSTATE_UINT32(spi, PXA2xxMMCIState),
+        VMSTATE_UINT32(cmdat, PXA2xxMMCIState),
+        VMSTATE_UINT32(resp_tout, PXA2xxMMCIState),
+        VMSTATE_UINT32(read_tout, PXA2xxMMCIState),
+        VMSTATE_INT32(blklen, PXA2xxMMCIState),
+        VMSTATE_INT32(numblk, PXA2xxMMCIState),
+        VMSTATE_UINT32(intmask, PXA2xxMMCIState),
+        VMSTATE_UINT32(intreq, PXA2xxMMCIState),
+        VMSTATE_INT32(cmd, PXA2xxMMCIState),
+        VMSTATE_UINT32(arg, PXA2xxMMCIState),
+        VMSTATE_INT32(cmdreq, PXA2xxMMCIState),
+        VMSTATE_INT32(active, PXA2xxMMCIState),
+        VMSTATE_INT32(bytesleft, PXA2xxMMCIState),
+        VMSTATE_UINT32(tx_start, PXA2xxMMCIState),
+        VMSTATE_UINT32(tx_len, PXA2xxMMCIState),
+        VMSTATE_UINT32(rx_start, PXA2xxMMCIState),
+        VMSTATE_UINT32(rx_len, PXA2xxMMCIState),
+        VMSTATE_UINT32(resp_len, PXA2xxMMCIState),
+        VMSTATE_VALIDATE("fifo size incorrect", pxa2xx_mmci_vmstate_validate),
+        VMSTATE_UINT8_ARRAY(tx_fifo, PXA2xxMMCIState, 64),
+        VMSTATE_UINT8_ARRAY(rx_fifo, PXA2xxMMCIState, 32),
+        VMSTATE_UINT16_ARRAY(resp_fifo, PXA2xxMMCIState, 9),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 #define MMC_STRPCL	0x00	/* MMC Clock Start/Stop register */
 #define MMC_STAT	0x04	/* MMC Status register */
 #define MMC_CLKRT	0x08	/* MMC Clock Rate register */
@@ -406,84 +451,6 @@ static const MemoryRegionOps pxa2xx_mmci_ops = {
     .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void pxa2xx_mmci_save(QEMUFile *f, void *opaque)
-{
-    PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
-    int i;
-
-    qemu_put_be32s(f, &s->status);
-    qemu_put_be32s(f, &s->clkrt);
-    qemu_put_be32s(f, &s->spi);
-    qemu_put_be32s(f, &s->cmdat);
-    qemu_put_be32s(f, &s->resp_tout);
-    qemu_put_be32s(f, &s->read_tout);
-    qemu_put_be32(f, s->blklen);
-    qemu_put_be32(f, s->numblk);
-    qemu_put_be32s(f, &s->intmask);
-    qemu_put_be32s(f, &s->intreq);
-    qemu_put_be32(f, s->cmd);
-    qemu_put_be32s(f, &s->arg);
-    qemu_put_be32(f, s->cmdreq);
-    qemu_put_be32(f, s->active);
-    qemu_put_be32(f, s->bytesleft);
-
-    qemu_put_byte(f, s->tx_len);
-    for (i = 0; i < s->tx_len; i ++)
-        qemu_put_byte(f, s->tx_fifo[(s->tx_start + i) & 63]);
-
-    qemu_put_byte(f, s->rx_len);
-    for (i = 0; i < s->rx_len; i ++)
-        qemu_put_byte(f, s->rx_fifo[(s->rx_start + i) & 31]);
-
-    qemu_put_byte(f, s->resp_len);
-    for (i = s->resp_len; i < 9; i ++)
-        qemu_put_be16s(f, &s->resp_fifo[i]);
-}
-
-static int pxa2xx_mmci_load(QEMUFile *f, void *opaque, int version_id)
-{
-    PXA2xxMMCIState *s = (PXA2xxMMCIState *) opaque;
-    int i;
-
-    qemu_get_be32s(f, &s->status);
-    qemu_get_be32s(f, &s->clkrt);
-    qemu_get_be32s(f, &s->spi);
-    qemu_get_be32s(f, &s->cmdat);
-    qemu_get_be32s(f, &s->resp_tout);
-    qemu_get_be32s(f, &s->read_tout);
-    s->blklen = qemu_get_be32(f);
-    s->numblk = qemu_get_be32(f);
-    qemu_get_be32s(f, &s->intmask);
-    qemu_get_be32s(f, &s->intreq);
-    s->cmd = qemu_get_be32(f);
-    qemu_get_be32s(f, &s->arg);
-    s->cmdreq = qemu_get_be32(f);
-    s->active = qemu_get_be32(f);
-    s->bytesleft = qemu_get_be32(f);
-
-    s->tx_len = qemu_get_byte(f);
-    s->tx_start = 0;
-    if (s->tx_len >= sizeof(s->tx_fifo) || s->tx_len < 0)
-        return -EINVAL;
-    for (i = 0; i < s->tx_len; i ++)
-        s->tx_fifo[i] = qemu_get_byte(f);
-
-    s->rx_len = qemu_get_byte(f);
-    s->rx_start = 0;
-    if (s->rx_len >= sizeof(s->rx_fifo) || s->rx_len < 0)
-        return -EINVAL;
-    for (i = 0; i < s->rx_len; i ++)
-        s->rx_fifo[i] = qemu_get_byte(f);
-
-    s->resp_len = qemu_get_byte(f);
-    if (s->resp_len > 9 || s->resp_len < 0)
-        return -EINVAL;
-    for (i = s->resp_len; i < 9; i ++)
-         qemu_get_be16s(f, &s->resp_fifo[i]);
-
-    return 0;
-}
-
 PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
                 hwaddr base,
                 BlockBackend *blk, qemu_irq irq,
@@ -557,13 +524,17 @@ static void pxa2xx_mmci_instance_init(Object *obj)
     qdev_init_gpio_out_named(dev, &s->rx_dma, "rx-dma", 1);
     qdev_init_gpio_out_named(dev, &s->tx_dma, "tx-dma", 1);
 
-    register_savevm(NULL, "pxa2xx_mmci", 0, 0,
-                    pxa2xx_mmci_save, pxa2xx_mmci_load, s);
-
     qbus_create_inplace(&s->sdbus, sizeof(s->sdbus),
                         TYPE_PXA2XX_MMCI_BUS, DEVICE(obj), "sd-bus");
 }
 
+static void pxa2xx_mmci_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->vmsd = &vmstate_pxa2xx_mmci;
+}
+
 static void pxa2xx_mmci_bus_class_init(ObjectClass *klass, void *data)
 {
     SDBusClass *sbc = SD_BUS_CLASS(klass);
@@ -577,6 +548,7 @@ static const TypeInfo pxa2xx_mmci_info = {
     .parent = TYPE_SYS_BUS_DEVICE,
     .instance_size = sizeof(PXA2xxMMCIState),
     .instance_init = pxa2xx_mmci_instance_init,
+    .class_init = pxa2xx_mmci_class_init,
 };
 
 static const TypeInfo pxa2xx_mmci_bus_info = {
-- 
1.9.1

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

* [Qemu-devel] [PATCH v3 10/10] hw/sd/pxa2xx_mmci: Add reset function
  2016-02-16 18:09 [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (8 preceding siblings ...)
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 09/10] hw/sd/pxa2xx_mmci: Convert to VMStateDescription Peter Maydell
@ 2016-02-16 18:09 ` Peter Maydell
  2016-02-18 11:48 ` [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
  10 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2016-02-16 18:09 UTC (permalink / raw)
  To: qemu-devel
  Cc: Edgar E. Iglesias, Alistair Francis, qemu-arm, Peter Crosthwaite

Add a reset function to the pxa2xx_mmci device; previously it had
no handling for system reset at all.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
 hw/sd/pxa2xx_mmci.c | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
index d9f5202..001d04e 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -511,6 +511,35 @@ void pxa2xx_mmci_handlers(PXA2xxMMCIState *s, qemu_irq readonly,
     pxa2xx_mmci_set_readonly(dev, sdbus_get_readonly(&s->sdbus));
 }
 
+static void pxa2xx_mmci_reset(DeviceState *d)
+{
+    PXA2xxMMCIState *s = PXA2XX_MMCI(d);
+
+    s->status = 0;
+    s->clkrt = 0;
+    s->spi = 0;
+    s->cmdat = 0;
+    s->resp_tout = 0;
+    s->read_tout = 0;
+    s->blklen = 0;
+    s->numblk = 0;
+    s->intmask = 0;
+    s->intreq = 0;
+    s->cmd = 0;
+    s->arg = 0;
+    s->active = 0;
+    s->bytesleft = 0;
+    s->tx_start = 0;
+    s->tx_len = 0;
+    s->rx_start = 0;
+    s->rx_len = 0;
+    s->resp_len = 0;
+    s->cmdreq = 0;
+    memset(s->tx_fifo, 0, sizeof(s->tx_fifo));
+    memset(s->rx_fifo, 0, sizeof(s->rx_fifo));
+    memset(s->resp_fifo, 0, sizeof(s->resp_fifo));
+}
+
 static void pxa2xx_mmci_instance_init(Object *obj)
 {
     PXA2xxMMCIState *s = PXA2XX_MMCI(obj);
@@ -533,6 +562,7 @@ static void pxa2xx_mmci_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->vmsd = &vmstate_pxa2xx_mmci;
+    dc->reset = pxa2xx_mmci_reset;
 }
 
 static void pxa2xx_mmci_bus_class_init(ObjectClass *klass, void *data)
-- 
1.9.1

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

* Re: [Qemu-devel] [PATCH v3 04/10] hw/sd: Add QOM bus which SD cards plug in to
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 04/10] hw/sd: Add QOM bus which SD cards plug in to Peter Maydell
@ 2016-02-17 17:34   ` Alistair Francis
  0 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2016-02-17 17:34 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Edgar E. Iglesias, Peter Crosthwaite, qemu-arm,
	qemu-devel@nongnu.org Developers, Alistair Francis

On Tue, Feb 16, 2016 at 10:09 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> Add a QOM bus for SD cards to plug in to.
>
> Note that since sd_enable() is used only by one board and there
> only as part of a broken implementation, we do not provide it in
> the SDBus API (but instead add a warning comment about the old
> function). Whoever converts OMAP and the nseries boards to QOM
> will need to either implement the card switch properly or move
> the enable hack into the OMAP MMC controller model.
>
> In the SDBus API, the old-style use of sd_set_cb to register some
> qemu_irqs for notification of card insertion and write-protect
> toggling is replaced with methods in the SDBusClass which the
> card calls on status changes and methods in the SDClass which
> the controller can call to find out the current status. The
> query methods will allow us to remove the abuse of the 'register
> irqs' API by controllers in their reset methods to trigger
> the card to tell them about the current status again.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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

Thanks,

Alistair

> ---
>  hw/sd/Makefile.objs |   2 +-
>  hw/sd/core.c        | 146 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/sd/sd.c          |  47 +++++++++++++++--
>  include/hw/sd/sd.h  |  62 ++++++++++++++++++++++
>  4 files changed, 252 insertions(+), 5 deletions(-)
>  create mode 100644 hw/sd/core.c
>
> diff --git a/hw/sd/Makefile.objs b/hw/sd/Makefile.objs
> index f1aed83..31c8330 100644
> --- a/hw/sd/Makefile.objs
> +++ b/hw/sd/Makefile.objs
> @@ -1,6 +1,6 @@
>  common-obj-$(CONFIG_PL181) += pl181.o
>  common-obj-$(CONFIG_SSI_SD) += ssi-sd.o
> -common-obj-$(CONFIG_SD) += sd.o
> +common-obj-$(CONFIG_SD) += sd.o core.o
>  common-obj-$(CONFIG_SDHCI) += sdhci.o
>
>  obj-$(CONFIG_MILKYMIST) += milkymist-memcard.o
> diff --git a/hw/sd/core.c b/hw/sd/core.c
> new file mode 100644
> index 0000000..14c2bdf
> --- /dev/null
> +++ b/hw/sd/core.c
> @@ -0,0 +1,146 @@
> +/*
> + * SD card bus interface code.
> + *
> + * Copyright (c) 2015 Linaro Limited
> + *
> + * Author:
> + *  Peter Maydell <peter.maydell@linaro.org>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2 or later, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + *
> + * You should have received a copy of the GNU General Public License along with
> + * this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "hw/qdev-core.h"
> +#include "sysemu/block-backend.h"
> +#include "hw/sd/sd.h"
> +
> +static SDState *get_card(SDBus *sdbus)
> +{
> +    /* We only ever have one child on the bus so just return it */
> +    BusChild *kid = QTAILQ_FIRST(&sdbus->qbus.children);
> +
> +    if (!kid) {
> +        return NULL;
> +    }
> +    return SD_CARD(kid->child);
> +}
> +
> +int sdbus_do_command(SDBus *sdbus, SDRequest *req, uint8_t *response)
> +{
> +    SDState *card = get_card(sdbus);
> +
> +    if (card) {
> +        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +
> +        return sc->do_command(card, req, response);
> +    }
> +
> +    return 0;
> +}
> +
> +void sdbus_write_data(SDBus *sdbus, uint8_t value)
> +{
> +    SDState *card = get_card(sdbus);
> +
> +    if (card) {
> +        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +
> +        sc->write_data(card, value);
> +    }
> +}
> +
> +uint8_t sdbus_read_data(SDBus *sdbus)
> +{
> +    SDState *card = get_card(sdbus);
> +
> +    if (card) {
> +        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +
> +        return sc->read_data(card);
> +    }
> +
> +    return 0;
> +}
> +
> +bool sdbus_data_ready(SDBus *sdbus)
> +{
> +    SDState *card = get_card(sdbus);
> +
> +    if (card) {
> +        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +
> +        return sc->data_ready(card);
> +    }
> +
> +    return false;
> +}
> +
> +bool sdbus_get_inserted(SDBus *sdbus)
> +{
> +    SDState *card = get_card(sdbus);
> +
> +    if (card) {
> +        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +
> +        return sc->get_inserted(card);
> +    }
> +
> +    return false;
> +}
> +
> +bool sdbus_get_readonly(SDBus *sdbus)
> +{
> +    SDState *card = get_card(sdbus);
> +
> +    if (card) {
> +        SDCardClass *sc = SD_CARD_GET_CLASS(card);
> +
> +        return sc->get_readonly(card);
> +    }
> +
> +    return false;
> +}
> +
> +void sdbus_set_inserted(SDBus *sdbus, bool inserted)
> +{
> +    SDBusClass *sbc = SD_BUS_GET_CLASS(sdbus);
> +    BusState *qbus = BUS(sdbus);
> +
> +    if (sbc->set_inserted) {
> +        sbc->set_inserted(qbus->parent, inserted);
> +    }
> +}
> +
> +void sdbus_set_readonly(SDBus *sdbus, bool readonly)
> +{
> +    SDBusClass *sbc = SD_BUS_GET_CLASS(sdbus);
> +    BusState *qbus = BUS(sdbus);
> +
> +    if (sbc->set_readonly) {
> +        sbc->set_readonly(qbus->parent, readonly);
> +    }
> +}
> +
> +static const TypeInfo sd_bus_info = {
> +    .name = TYPE_SD_BUS,
> +    .parent = TYPE_BUS,
> +    .instance_size = sizeof(SDBus),
> +    .class_size = sizeof(SDBusClass),
> +};
> +
> +static void sd_bus_register_types(void)
> +{
> +    type_register_static(&sd_bus_info);
> +}
> +
> +type_init(sd_bus_register_types)
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index edd4c82..8902cd8 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -30,6 +30,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "hw/qdev.h"
>  #include "hw/hw.h"
>  #include "sysemu/block-backend.h"
>  #include "hw/sd/sd.h"
> @@ -431,14 +432,41 @@ static void sd_reset(DeviceState *dev)
>      sd->expecting_acmd = false;
>  }
>
> +static bool sd_get_inserted(SDState *sd)
> +{
> +    return blk_is_inserted(sd->blk);
> +}
> +
> +static bool sd_get_readonly(SDState *sd)
> +{
> +    return sd->wp_switch;
> +}
> +
>  static void sd_cardchange(void *opaque, bool load)
>  {
>      SDState *sd = opaque;
> +    DeviceState *dev = DEVICE(sd);
> +    SDBus *sdbus = SD_BUS(qdev_get_parent_bus(dev));
> +    bool inserted = sd_get_inserted(sd);
> +    bool readonly = sd_get_readonly(sd);
>
> -    qemu_set_irq(sd->inserted_cb, blk_is_inserted(sd->blk));
> -    if (blk_is_inserted(sd->blk)) {
> -        sd_reset(DEVICE(sd));
> -        qemu_set_irq(sd->readonly_cb, sd->wp_switch);
> +    if (inserted) {
> +        sd_reset(dev);
> +    }
> +
> +    /* The IRQ notification is for legacy non-QOM SD controller devices;
> +     * QOMified controllers use the SDBus APIs.
> +     */
> +    if (sdbus) {
> +        sdbus_set_inserted(sdbus, inserted);
> +        if (inserted) {
> +            sdbus_set_readonly(sdbus, readonly);
> +        }
> +    } else {
> +        qemu_set_irq(sd->inserted_cb, inserted);
> +        if (inserted) {
> +            qemu_set_irq(sd->readonly_cb, readonly);
> +        }
>      }
>  }
>
> @@ -1803,17 +1831,28 @@ static Property sd_properties[] = {
>  static void sd_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
> +    SDCardClass *sc = SD_CARD_CLASS(klass);
>
>      dc->realize = sd_realize;
>      dc->props = sd_properties;
>      dc->vmsd = &sd_vmstate;
>      dc->reset = sd_reset;
> +    dc->bus_type = TYPE_SD_BUS;
> +
> +    sc->do_command = sd_do_command;
> +    sc->write_data = sd_write_data;
> +    sc->read_data = sd_read_data;
> +    sc->data_ready = sd_data_ready;
> +    sc->enable = sd_enable;
> +    sc->get_inserted = sd_get_inserted;
> +    sc->get_readonly = sd_get_readonly;
>  }
>
>  static const TypeInfo sd_info = {
>      .name = TYPE_SD_CARD,
>      .parent = TYPE_DEVICE,
>      .instance_size = sizeof(SDState),
> +    .class_size = sizeof(SDCardClass),
>      .class_init = sd_class_init,
>      .instance_init = sd_instance_init,
>  };
> diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
> index 404d589..d5d273a 100644
> --- a/include/hw/sd/sd.h
> +++ b/include/hw/sd/sd.h
> @@ -67,10 +67,51 @@ typedef struct {
>  } SDRequest;
>
>  typedef struct SDState SDState;
> +typedef struct SDBus SDBus;
>
>  #define TYPE_SD_CARD "sd-card"
>  #define SD_CARD(obj) OBJECT_CHECK(SDState, (obj), TYPE_SD_CARD)
> +#define SD_CARD_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(SDCardClass, (klass), TYPE_SD_CARD)
> +#define SD_CARD_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(SDCardClass, (obj), TYPE_SD_CARD)
>
> +typedef struct {
> +    /*< private >*/
> +    DeviceClass parent_class;
> +    /*< public >*/
> +
> +    int (*do_command)(SDState *sd, SDRequest *req, uint8_t *response);
> +    void (*write_data)(SDState *sd, uint8_t value);
> +    uint8_t (*read_data)(SDState *sd);
> +    bool (*data_ready)(SDState *sd);
> +    void (*enable)(SDState *sd, bool enable);
> +    bool (*get_inserted)(SDState *sd);
> +    bool (*get_readonly)(SDState *sd);
> +} SDCardClass;
> +
> +#define TYPE_SD_BUS "sd-bus"
> +#define SD_BUS(obj) OBJECT_CHECK(SDBus, (obj), TYPE_SD_BUS)
> +#define SD_BUS_CLASS(klass) OBJECT_CLASS_CHECK(SDBusClass, (klass), TYPE_SD_BUS)
> +#define SD_BUS_GET_CLASS(obj) OBJECT_GET_CLASS(SDBusClass, (obj), TYPE_SD_BUS)
> +
> +struct SDBus {
> +    BusState qbus;
> +};
> +
> +typedef struct {
> +    /*< private >*/
> +    BusClass parent_class;
> +    /*< public >*/
> +
> +    /* These methods are called by the SD device to notify the controller
> +     * when the card insertion or readonly status changes
> +     */
> +    void (*set_inserted)(DeviceState *dev, bool inserted);
> +    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);
> @@ -78,6 +119,27 @@ 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)
> + */
> +int sdbus_do_command(SDBus *sd, SDRequest *req, uint8_t *response);
> +void sdbus_write_data(SDBus *sd, uint8_t value);
> +uint8_t sdbus_read_data(SDBus *sd);
> +bool sdbus_data_ready(SDBus *sd);
> +bool sdbus_get_inserted(SDBus *sd);
> +bool sdbus_get_readonly(SDBus *sd);
> +
> +/* Functions to be used by SD devices to report back to qdevified controllers */
> +void sdbus_set_inserted(SDBus *sd, bool inserted);
> +void sdbus_set_readonly(SDBus *sd, bool inserted);
> +
>  #endif /* __hw_sd_h */
> --
> 1.9.1
>
>

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

* Re: [Qemu-devel] [PATCH v3 02/10] hw/sd/sd.c: QOMify
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 02/10] hw/sd/sd.c: QOMify Peter Maydell
@ 2016-02-17 17:34   ` Alistair Francis
  0 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2016-02-17 17:34 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Edgar E. Iglesias, Peter Crosthwaite, qemu-arm,
	qemu-devel@nongnu.org Developers, Alistair Francis

On Tue, Feb 16, 2016 at 10:09 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> Turn the SD card into a QOM device.
> This conversion only changes the device itself; the various
> functions which are effectively methods on the device are not
> touched at this point.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>

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

Thanks,

Alistair

> ---
>  hw/sd/sd.c         | 99 ++++++++++++++++++++++++++++++++++++++++++------------
>  include/hw/sd/sd.h |  3 ++
>  2 files changed, 80 insertions(+), 22 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index dd614b0..1681728 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -34,6 +34,8 @@
>  #include "sysemu/block-backend.h"
>  #include "hw/sd/sd.h"
>  #include "qemu/bitmap.h"
> +#include "hw/qdev-properties.h"
> +#include "qemu/error-report.h"
>
>  //#define DEBUG_SD 1
>
> @@ -78,6 +80,8 @@ enum SDCardStates {
>  };
>
>  struct SDState {
> +    DeviceState parent_obj;
> +
>      uint32_t mode;    /* current card mode, one of SDCardModes */
>      int32_t state;    /* current card state, one of SDCardStates */
>      uint32_t ocr;
> @@ -473,34 +477,26 @@ static const VMStateDescription sd_vmstate = {
>      }
>  };
>
> -/* 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
> -   is asserted.  */
> +/* Legacy initialization function for use by non-qdevified callers */
>  SDState *sd_init(BlockBackend *blk, bool is_spi)
>  {
> -    SDState *sd;
> +    DeviceState *dev;
> +    Error *err = NULL;
>
> -    if (blk && blk_is_read_only(blk)) {
> -        fprintf(stderr, "sd_init: Cannot use read-only drive\n");
> +    dev = qdev_create(NULL, TYPE_SD_CARD);
> +    qdev_prop_set_drive(dev, "drive", blk, &err);
> +    if (err) {
> +        error_report("sd_init failed: %s", error_get_pretty(err));
>          return NULL;
>      }
> -
> -    sd = (SDState *) g_malloc0(sizeof(SDState));
> -    sd->buf = blk_blockalign(blk, 512);
> -    sd->spi = is_spi;
> -    sd->enable = true;
> -    sd->blk = blk;
> -    sd_reset(sd);
> -    if (sd->blk) {
> -        /* Attach dev if not already attached.  (This call ignores an
> -         * error return code if sd->blk is already attached.) */
> -        /* FIXME ignoring blk_attach_dev() failure is dangerously brittle */
> -        blk_attach_dev(sd->blk, sd);
> -        blk_set_dev_ops(sd->blk, &sd_block_ops, sd);
> +    qdev_prop_set_bit(dev, "spi", is_spi);
> +    object_property_set_bool(OBJECT(dev), true, "realized", &err);
> +    if (err) {
> +        error_report("sd_init failed: %s", error_get_pretty(err));
> +        return NULL;
>      }
> -    vmstate_register(NULL, -1, &sd_vmstate, sd);
> -    return sd;
> +
> +    return SD_CARD(dev);
>  }
>
>  void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
> @@ -1769,3 +1765,62 @@ void sd_enable(SDState *sd, bool enable)
>  {
>      sd->enable = enable;
>  }
> +
> +static void sd_instance_init(Object *obj)
> +{
> +    SDState *sd = SD_CARD(obj);
> +
> +    sd->enable = true;
> +}
> +
> +static void sd_realize(DeviceState *dev, Error **errp)
> +{
> +    SDState *sd = SD_CARD(dev);
> +
> +    if (sd->blk && blk_is_read_only(sd->blk)) {
> +        error_setg(errp, "Cannot use read-only drive as SD card");
> +        return;
> +    }
> +
> +    sd->buf = blk_blockalign(sd->blk, 512);
> +
> +    if (sd->blk) {
> +        blk_set_dev_ops(sd->blk, &sd_block_ops, sd);
> +    }
> +
> +    sd_reset(sd);
> +}
> +
> +static Property sd_properties[] = {
> +    DEFINE_PROP_DRIVE("drive", SDState, blk),
> +    /* 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
> +     * is asserted.  */
> +    DEFINE_PROP_BOOL("spi", SDState, spi, false),
> +    DEFINE_PROP_END_OF_LIST()
> +};
> +
> +static void sd_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = sd_realize;
> +    dc->props = sd_properties;
> +    dc->vmsd = &sd_vmstate;
> +}
> +
> +static const TypeInfo sd_info = {
> +    .name = TYPE_SD_CARD,
> +    .parent = TYPE_DEVICE,
> +    .instance_size = sizeof(SDState),
> +    .class_init = sd_class_init,
> +    .instance_init = sd_instance_init,
> +};
> +
> +static void sd_register_types(void)
> +{
> +    type_register_static(&sd_info);
> +}
> +
> +type_init(sd_register_types)
> diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
> index 79adb5b..404d589 100644
> --- a/include/hw/sd/sd.h
> +++ b/include/hw/sd/sd.h
> @@ -68,6 +68,9 @@ typedef struct {
>
>  typedef struct SDState SDState;
>
> +#define TYPE_SD_CARD "sd-card"
> +#define SD_CARD(obj) OBJECT_CHECK(SDState, (obj), TYPE_SD_CARD)
> +
>  SDState *sd_init(BlockBackend *bs, bool is_spi);
>  int sd_do_command(SDState *sd, SDRequest *req,
>                    uint8_t *response);
> --
> 1.9.1
>
>

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

* Re: [Qemu-devel] [PATCH v3 06/10] sdhci_sysbus: Create SD card device in users, not the device itself
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 06/10] sdhci_sysbus: Create SD card device in users, not the device itself Peter Maydell
@ 2016-02-17 18:01   ` Alistair Francis
  0 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2016-02-17 18:01 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Edgar E. Iglesias, Peter Crosthwaite, qemu-arm,
	qemu-devel@nongnu.org Developers, Alistair Francis

On Tue, Feb 16, 2016 at 10:09 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> Move the creation of the SD card device from the sdhci_sysbus
> device itself into the boards that create these devices.
> This allows us to remove the cannot_instantiate_with_device_add
> notation because we no longer call drive_get_next in the device
> model.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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

Thanks,

Alistair

> ---
>  hw/arm/xilinx_zynq.c | 17 ++++++++++++++++-
>  hw/arm/xlnx-ep108.c  | 21 +++++++++++++++++++++
>  hw/arm/xlnx-zynqmp.c |  8 ++++++++
>  hw/sd/sdhci.c        | 25 -------------------------
>  4 files changed, 45 insertions(+), 26 deletions(-)
>
> diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
> index 66e7f27..a35983a 100644
> --- a/hw/arm/xilinx_zynq.c
> +++ b/hw/arm/xilinx_zynq.c
> @@ -28,6 +28,7 @@
>  #include "hw/misc/zynq-xadc.h"
>  #include "hw/ssi/ssi.h"
>  #include "qemu/error-report.h"
> +#include "hw/sd/sd.h"
>
>  #define NUM_SPI_FLASHES 4
>  #define NUM_QSPI_FLASHES 2
> @@ -154,8 +155,10 @@ static void zynq_init(MachineState *machine)
>      MemoryRegion *address_space_mem = get_system_memory();
>      MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
>      MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
> -    DeviceState *dev;
> +    DeviceState *dev, *carddev;
>      SysBusDevice *busdev;
> +    DriveInfo *di;
> +    BlockBackend *blk;
>      qemu_irq pic[64];
>      int n;
>
> @@ -245,11 +248,23 @@ static void zynq_init(MachineState *machine)
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0100000);
>      sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[56-IRQ_OFFSET]);
>
> +    di = drive_get_next(IF_SD);
> +    blk = di ? blk_by_legacy_dinfo(di) : NULL;
> +    carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
> +    qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
> +    object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal);
> +
>      dev = qdev_create(NULL, "generic-sdhci");
>      qdev_init_nofail(dev);
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0101000);
>      sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[79-IRQ_OFFSET]);
>
> +    di = drive_get_next(IF_SD);
> +    blk = di ? blk_by_legacy_dinfo(di) : NULL;
> +    carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
> +    qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
> +    object_property_set_bool(OBJECT(carddev), true, "realized", &error_fatal);
> +
>      dev = qdev_create(NULL, TYPE_ZYNQ_XADC);
>      qdev_init_nofail(dev);
>      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8007100);
> diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
> index 0de132a..a1bd283 100644
> --- a/hw/arm/xlnx-ep108.c
> +++ b/hw/arm/xlnx-ep108.c
> @@ -59,6 +59,27 @@ static void xlnx_ep108_init(MachineState *machine)
>
>      object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
>
> +    /* Create and plug in the SD cards */
> +    for (i = 0; i < XLNX_ZYNQMP_NUM_SDHCI; i++) {
> +        BusState *bus;
> +        DriveInfo *di = drive_get_next(IF_SD);
> +        BlockBackend *blk = di ? blk_by_legacy_dinfo(di) : NULL;
> +        DeviceState *carddev;
> +        char *bus_name;
> +
> +        bus_name = g_strdup_printf("sd-bus%d", i);
> +        bus = qdev_get_child_bus(DEVICE(&s->soc), bus_name);
> +        g_free(bus_name);
> +        if (!bus) {
> +            error_report("No SD bus found for SD card %d", i);
> +            exit(1);
> +        }
> +        carddev = qdev_create(bus, TYPE_SD_CARD);
> +        qdev_prop_set_drive(carddev, "drive", blk, &error_fatal);
> +        object_property_set_bool(OBJECT(carddev), true, "realized",
> +                                 &error_fatal);
> +    }
> +
>      for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
>          SSIBus *spi_bus;
>          DeviceState *flash_dev;
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 1508d08..4fbb635 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -327,6 +327,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>      sysbus_connect_irq(SYS_BUS_DEVICE(&s->sata), 0, gic_spi[SATA_INTR]);
>
>      for (i = 0; i < XLNX_ZYNQMP_NUM_SDHCI; i++) {
> +        char *bus_name;
> +
>          object_property_set_bool(OBJECT(&s->sdhci[i]), true,
>                                   "realized", &err);
>          if (err) {
> @@ -337,6 +339,12 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>                          sdhci_addr[i]);
>          sysbus_connect_irq(SYS_BUS_DEVICE(&s->sdhci[i]), 0,
>                             gic_spi[sdhci_intr[i]]);
> +        /* Alias controller SD bus to the SoC itself */
> +        bus_name = g_strdup_printf("sd-bus%d", i);
> +        object_property_add_alias(OBJECT(s), bus_name,
> +                                  OBJECT(&s->sdhci[i]), "sd-bus",
> +                                  &error_abort);
> +        g_free(bus_name);
>      }
>
>      for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 396dd10..73e7c87 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -1296,29 +1296,6 @@ static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp)
>  {
>      SDHCIState *s = SYSBUS_SDHCI(dev);
>      SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> -    DriveInfo *di;
> -    BlockBackend *blk;
> -    DeviceState *carddev;
> -    Error *err = NULL;
> -
> -    /* Create and plug in the sd card.
> -     * FIXME: this should be done by the users of this device so we
> -     * do not use drive_get_next() here.
> -     */
> -    di = drive_get_next(IF_SD);
> -    blk = di ? blk_by_legacy_dinfo(di) : NULL;
> -
> -    carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
> -    qdev_prop_set_drive(carddev, "drive", blk, &err);
> -    if (err) {
> -        error_propagate(errp, err);
> -        return;
> -    }
> -    object_property_set_bool(OBJECT(carddev), true, "realized", &err);
> -    if (err) {
> -        error_propagate(errp, err);
> -        return;
> -    }
>
>      s->buf_maxsz = sdhci_get_fifolen(s);
>      s->fifo_buffer = g_malloc0(s->buf_maxsz);
> @@ -1335,8 +1312,6 @@ static void sdhci_sysbus_class_init(ObjectClass *klass, void *data)
>      dc->vmsd = &sdhci_vmstate;
>      dc->props = sdhci_sysbus_properties;
>      dc->realize = sdhci_sysbus_realize;
> -    /* Reason: instance_init() method uses drive_get_next() */
> -    dc->cannot_instantiate_with_device_add_yet = true;
>  }
>
>  static const TypeInfo sdhci_sysbus_info = {
> --
> 1.9.1
>
>

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

* Re: [Qemu-devel] [PATCH v3 05/10] hw/sd/sdhci.c: Update to use SDBus APIs
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 05/10] hw/sd/sdhci.c: Update to use SDBus APIs Peter Maydell
@ 2016-02-17 18:01   ` Alistair Francis
  0 siblings, 0 replies; 16+ messages in thread
From: Alistair Francis @ 2016-02-17 18:01 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Edgar E. Iglesias, Peter Crosthwaite, qemu-arm,
	qemu-devel@nongnu.org Developers, Alistair Francis

On Tue, Feb 16, 2016 at 10:09 AM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> Update the SDHCI code to use the new SDBus APIs.
>
> This commit introduces the new command line options required
> to connect a disk to sdhci-pci:
>
>  -device sdhci-pci -drive id=mydrive,[...] -device sd,drive=mydrive
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

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

Thanks,

Alistair

> ---
>  hw/sd/sdhci.c         | 97 ++++++++++++++++++++++++++++++++++++---------------
>  include/hw/sd/sdhci.h |  3 +-
>  2 files changed, 69 insertions(+), 31 deletions(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 3d1eb85..396dd10 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -55,6 +55,9 @@
>          } \
>      } while (0)
>
> +#define TYPE_SDHCI_BUS "sdhci-bus"
> +#define SDHCI_BUS(obj) OBJECT_CHECK(SDBus, (obj), TYPE_SDHCI_BUS)
> +
>  /* Default SD/MMC host controller features information, which will be
>   * presented in CAPABILITIES register of generic SD host controller at reset.
>   * If not stated otherwise:
> @@ -145,9 +148,9 @@ static void sdhci_raise_insertion_irq(void *opaque)
>      }
>  }
>
> -static void sdhci_insert_eject_cb(void *opaque, int irq, int level)
> +static void sdhci_set_inserted(DeviceState *dev, bool level)
>  {
> -    SDHCIState *s = (SDHCIState *)opaque;
> +    SDHCIState *s = (SDHCIState *)dev;
>      DPRINT_L1("Card state changed: %s!\n", level ? "insert" : "eject");
>
>      if ((s->norintsts & SDHC_NIS_REMOVE) && level) {
> @@ -172,9 +175,9 @@ static void sdhci_insert_eject_cb(void *opaque, int irq, int level)
>      }
>  }
>
> -static void sdhci_card_readonly_cb(void *opaque, int irq, int level)
> +static void sdhci_set_readonly(DeviceState *dev, bool level)
>  {
> -    SDHCIState *s = (SDHCIState *)opaque;
> +    SDHCIState *s = (SDHCIState *)dev;
>
>      if (level) {
>          s->prnsts &= ~SDHC_WRITE_PROTECT;
> @@ -186,6 +189,8 @@ static void sdhci_card_readonly_cb(void *opaque, int irq, int level)
>
>  static void sdhci_reset(SDHCIState *s)
>  {
> +    DeviceState *dev = DEVICE(s);
> +
>      timer_del(s->insert_timer);
>      timer_del(s->transfer_timer);
>      /* Set all registers to 0. Capabilities registers are not cleared
> @@ -194,8 +199,11 @@ static void sdhci_reset(SDHCIState *s)
>      memset(&s->sdmasysad, 0, (uintptr_t)&s->capareg - (uintptr_t)&s->sdmasysad);
>
>      if (!s->noeject_quirk) {
> -        sd_set_cb(s->card, s->ro_cb, s->eject_cb);
> +        /* Reset other state based on current card insertion/readonly status */
> +        sdhci_set_inserted(dev, sdbus_get_inserted(&s->sdbus));
> +        sdhci_set_readonly(dev, sdbus_get_readonly(&s->sdbus));
>      }
> +
>      s->data_count = 0;
>      s->stopped_state = sdhc_not_stopped;
>  }
> @@ -213,7 +221,7 @@ static void sdhci_send_command(SDHCIState *s)
>      request.cmd = s->cmdreg >> 8;
>      request.arg = s->argument;
>      DPRINT_L1("sending CMD%u ARG[0x%08x]\n", request.cmd, request.arg);
> -    rlen = sd_do_command(s->card, &request, response);
> +    rlen = sdbus_do_command(&s->sdbus, &request, response);
>
>      if (s->cmdreg & SDHC_CMD_RESPONSE) {
>          if (rlen == 4) {
> @@ -269,7 +277,7 @@ static void sdhci_end_transfer(SDHCIState *s)
>          request.cmd = 0x0C;
>          request.arg = 0;
>          DPRINT_L1("Automatically issue CMD%d %08x\n", request.cmd, request.arg);
> -        sd_do_command(s->card, &request, response);
> +        sdbus_do_command(&s->sdbus, &request, response);
>          /* Auto CMD12 response goes to the upper Response register */
>          s->rspreg[3] = (response[0] << 24) | (response[1] << 16) |
>                  (response[2] << 8) | response[3];
> @@ -301,7 +309,7 @@ static void sdhci_read_block_from_card(SDHCIState *s)
>      }
>
>      for (index = 0; index < (s->blksize & 0x0fff); index++) {
> -        s->fifo_buffer[index] = sd_read_data(s->card);
> +        s->fifo_buffer[index] = sdbus_read_data(&s->sdbus);
>      }
>
>      /* New data now available for READ through Buffer Port Register */
> @@ -394,7 +402,7 @@ static void sdhci_write_block_to_card(SDHCIState *s)
>      }
>
>      for (index = 0; index < (s->blksize & 0x0fff); index++) {
> -        sd_write_data(s->card, s->fifo_buffer[index]);
> +        sdbus_write_data(&s->sdbus, s->fifo_buffer[index]);
>      }
>
>      /* Next data can be written through BUFFER DATORT register */
> @@ -476,7 +484,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
>          while (s->blkcnt) {
>              if (s->data_count == 0) {
>                  for (n = 0; n < block_size; n++) {
> -                    s->fifo_buffer[n] = sd_read_data(s->card);
> +                    s->fifo_buffer[n] = sdbus_read_data(&s->sdbus);
>                  }
>              }
>              begin = s->data_count;
> @@ -517,7 +525,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
>              s->sdmasysad += s->data_count - begin;
>              if (s->data_count == block_size) {
>                  for (n = 0; n < block_size; n++) {
> -                    sd_write_data(s->card, s->fifo_buffer[n]);
> +                    sdbus_write_data(&s->sdbus, s->fifo_buffer[n]);
>                  }
>                  s->data_count = 0;
>                  if (s->trnmod & SDHC_TRNS_BLK_CNT_EN) {
> @@ -549,7 +557,7 @@ static void sdhci_sdma_transfer_single_block(SDHCIState *s)
>
>      if (s->trnmod & SDHC_TRNS_READ) {
>          for (n = 0; n < datacnt; n++) {
> -            s->fifo_buffer[n] = sd_read_data(s->card);
> +            s->fifo_buffer[n] = sdbus_read_data(&s->sdbus);
>          }
>          dma_memory_write(&address_space_memory, s->sdmasysad, s->fifo_buffer,
>                           datacnt);
> @@ -557,7 +565,7 @@ static void sdhci_sdma_transfer_single_block(SDHCIState *s)
>          dma_memory_read(&address_space_memory, s->sdmasysad, s->fifo_buffer,
>                          datacnt);
>          for (n = 0; n < datacnt; n++) {
> -            sd_write_data(s->card, s->fifo_buffer[n]);
> +            sdbus_write_data(&s->sdbus, s->fifo_buffer[n]);
>          }
>      }
>
> @@ -661,7 +669,7 @@ static void sdhci_do_adma(SDHCIState *s)
>                  while (length) {
>                      if (s->data_count == 0) {
>                          for (n = 0; n < block_size; n++) {
> -                            s->fifo_buffer[n] = sd_read_data(s->card);
> +                            s->fifo_buffer[n] = sdbus_read_data(&s->sdbus);
>                          }
>                      }
>                      begin = s->data_count;
> @@ -702,7 +710,7 @@ static void sdhci_do_adma(SDHCIState *s)
>                      dscr.addr += s->data_count - begin;
>                      if (s->data_count == block_size) {
>                          for (n = 0; n < block_size; n++) {
> -                            sd_write_data(s->card, s->fifo_buffer[n]);
> +                            sdbus_write_data(&s->sdbus, s->fifo_buffer[n]);
>                          }
>                          s->data_count = 0;
>                          if (s->trnmod & SDHC_TRNS_BLK_CNT_EN) {
> @@ -816,7 +824,7 @@ static void sdhci_data_transfer(void *opaque)
>              break;
>          }
>      } else {
> -        if ((s->trnmod & SDHC_TRNS_READ) && sd_data_ready(s->card)) {
> +        if ((s->trnmod & SDHC_TRNS_READ) && sdbus_data_ready(&s->sdbus)) {
>              s->prnsts |= SDHC_DOING_READ | SDHC_DATA_INHIBIT |
>                      SDHC_DAT_LINE_ACTIVE;
>              sdhci_read_block_from_card(s);
> @@ -1153,15 +1161,10 @@ static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
>      }
>  }
>
> -static void sdhci_initfn(SDHCIState *s, BlockBackend *blk)
> +static void sdhci_initfn(SDHCIState *s)
>  {
> -    s->card = sd_init(blk, false);
> -    if (s->card == NULL) {
> -        exit(1);
> -    }
> -    s->eject_cb = qemu_allocate_irq(sdhci_insert_eject_cb, s, 0);
> -    s->ro_cb = qemu_allocate_irq(sdhci_card_readonly_cb, s, 0);
> -    sd_set_cb(s->card, s->ro_cb, s->eject_cb);
> +    qbus_create_inplace(&s->sdbus, sizeof(s->sdbus),
> +                        TYPE_SDHCI_BUS, DEVICE(s), "sd-bus");
>
>      s->insert_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_raise_insertion_irq, s);
>      s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_data_transfer, s);
> @@ -1231,7 +1234,7 @@ static void sdhci_pci_realize(PCIDevice *dev, Error **errp)
>      SDHCIState *s = PCI_SDHCI(dev);
>      dev->config[PCI_CLASS_PROG] = 0x01; /* Standard Host supported DMA */
>      dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
> -    sdhci_initfn(s, s->blk);
> +    sdhci_initfn(s);
>      s->buf_maxsz = sdhci_get_fifolen(s);
>      s->fifo_buffer = g_malloc0(s->buf_maxsz);
>      s->irq = pci_allocate_irq(dev);
> @@ -1279,11 +1282,8 @@ static Property sdhci_sysbus_properties[] = {
>  static void sdhci_sysbus_init(Object *obj)
>  {
>      SDHCIState *s = SYSBUS_SDHCI(obj);
> -    DriveInfo *di;
>
> -    /* FIXME use a qdev drive property instead of drive_get_next() */
> -    di = drive_get_next(IF_SD);
> -    sdhci_initfn(s, di ? blk_by_legacy_dinfo(di) : NULL);
> +    sdhci_initfn(s);
>  }
>
>  static void sdhci_sysbus_finalize(Object *obj)
> @@ -1296,6 +1296,29 @@ static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp)
>  {
>      SDHCIState *s = SYSBUS_SDHCI(dev);
>      SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +    DriveInfo *di;
> +    BlockBackend *blk;
> +    DeviceState *carddev;
> +    Error *err = NULL;
> +
> +    /* Create and plug in the sd card.
> +     * FIXME: this should be done by the users of this device so we
> +     * do not use drive_get_next() here.
> +     */
> +    di = drive_get_next(IF_SD);
> +    blk = di ? blk_by_legacy_dinfo(di) : NULL;
> +
> +    carddev = qdev_create(qdev_get_child_bus(dev, "sd-bus"), TYPE_SD_CARD);
> +    qdev_prop_set_drive(carddev, "drive", blk, &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
> +    object_property_set_bool(OBJECT(carddev), true, "realized", &err);
> +    if (err) {
> +        error_propagate(errp, err);
> +        return;
> +    }
>
>      s->buf_maxsz = sdhci_get_fifolen(s);
>      s->fifo_buffer = g_malloc0(s->buf_maxsz);
> @@ -1325,10 +1348,26 @@ static const TypeInfo sdhci_sysbus_info = {
>      .class_init = sdhci_sysbus_class_init,
>  };
>
> +static void sdhci_bus_class_init(ObjectClass *klass, void *data)
> +{
> +    SDBusClass *sbc = SD_BUS_CLASS(klass);
> +
> +    sbc->set_inserted = sdhci_set_inserted;
> +    sbc->set_readonly = sdhci_set_readonly;
> +}
> +
> +static const TypeInfo sdhci_bus_info = {
> +    .name = TYPE_SDHCI_BUS,
> +    .parent = TYPE_SD_BUS,
> +    .instance_size = sizeof(SDBus),
> +    .class_init = sdhci_bus_class_init,
> +};
> +
>  static void sdhci_register_types(void)
>  {
>      type_register_static(&sdhci_pci_info);
>      type_register_static(&sdhci_sysbus_info);
> +    type_register_static(&sdhci_bus_info);
>  }
>
>  type_init(sdhci_register_types)
> diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
> index ffd1f80..607a83e 100644
> --- a/include/hw/sd/sdhci.h
> +++ b/include/hw/sd/sdhci.h
> @@ -37,9 +37,8 @@ typedef struct SDHCIState {
>          PCIDevice pcidev;
>          SysBusDevice busdev;
>      };
> -    SDState *card;
> +    SDBus sdbus;
>      MemoryRegion iomem;
> -    BlockBackend *blk;
>
>      QEMUTimer *insert_timer;       /* timer for 'changing' sd card. */
>      QEMUTimer *transfer_timer;
> --
> 1.9.1
>
>

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

* Re: [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci)
  2016-02-16 18:09 [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (9 preceding siblings ...)
  2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 10/10] hw/sd/pxa2xx_mmci: Add reset function Peter Maydell
@ 2016-02-18 11:48 ` Peter Maydell
  10 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2016-02-18 11:48 UTC (permalink / raw)
  To: QEMU Developers
  Cc: Edgar E. Iglesias, Peter Crosthwaite, qemu-arm, Alistair Francis

On 16 February 2016 at 18:09, Peter Maydell <peter.maydell@linaro.org> wrote:
> This series attempts to QOMify sd.c (the actual SD card model),
> including a proper QOM bus between the controller and the card.



Applied to target-arm.next, thanks.

-- PMM

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

end of thread, other threads:[~2016-02-18 11:49 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-16 18:09 [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 01/10] hw/sd/sdhci.c: Remove x-drive property Peter Maydell
2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 02/10] hw/sd/sd.c: QOMify Peter Maydell
2016-02-17 17:34   ` Alistair Francis
2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 03/10] hw/sd/sd.c: Convert sd_reset() function into Device reset method Peter Maydell
2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 04/10] hw/sd: Add QOM bus which SD cards plug in to Peter Maydell
2016-02-17 17:34   ` Alistair Francis
2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 05/10] hw/sd/sdhci.c: Update to use SDBus APIs Peter Maydell
2016-02-17 18:01   ` Alistair Francis
2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 06/10] sdhci_sysbus: Create SD card device in users, not the device itself Peter Maydell
2016-02-17 18:01   ` Alistair Francis
2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 07/10] hw/sd/pxa2xx_mmci: convert to SysBusDevice object Peter Maydell
2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 08/10] hw/sd/pxa2xx_mmci: Update to use new SDBus APIs Peter Maydell
2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 09/10] hw/sd/pxa2xx_mmci: Convert to VMStateDescription Peter Maydell
2016-02-16 18:09 ` [Qemu-devel] [PATCH v3 10/10] hw/sd/pxa2xx_mmci: Add reset function Peter Maydell
2016-02-18 11:48 ` [Qemu-devel] [PATCH v3 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).