qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci)
@ 2016-01-22 16:16 Peter Maydell
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 01/10] hw/sd/sdhci.c: Remove x-drive property Peter Maydell
                   ` (11 more replies)
  0 siblings, 12 replies; 17+ messages in thread
From: Peter Maydell @ 2016-01-22 16:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Crosthwaite, Sai Pavan Boddu, patches, Markus Armbruster,
	Alistair Francis, Kevin O'Connor, qemu-arm,
	Edgar E. Iglesias, Paolo Bonzini

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

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

If you want 'change' and 'eject' to work in the monitor you need to
base on top of Max's patches fixing that regression.
Git branch with that setup available at:
https://git.linaro.org/people/peter.maydell/qemu-arm.git pxa-mmci-qomify

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            | 149 +++++++++++++++++++-----
 hw/sd/sdhci.c         |  82 ++++++++------
 include/hw/sd/sd.h    |  65 +++++++++++
 include/hw/sd/sdhci.h |   3 +-
 10 files changed, 623 insertions(+), 176 deletions(-)
 create mode 100644 hw/sd/core.c

-- 
1.9.1

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

* [Qemu-devel] [PATCH v2 01/10] hw/sd/sdhci.c: Remove x-drive property
  2016-01-22 16:16 [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
@ 2016-01-22 16:16 ` Peter Maydell
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 02/10] hw/sd/sd.c: QOMify Peter Maydell
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-01-22 16:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Crosthwaite, Sai Pavan Boddu, patches, Markus Armbruster,
	Alistair Francis, Kevin O'Connor, qemu-arm,
	Edgar E. Iglesias, Paolo Bonzini

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 7acb4d7..a692eee 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] 17+ messages in thread

* [Qemu-devel] [PATCH v2 02/10] hw/sd/sd.c: QOMify
  2016-01-22 16:16 [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 01/10] hw/sd/sdhci.c: Remove x-drive property Peter Maydell
@ 2016-01-22 16:16 ` Peter Maydell
  2016-02-08 23:06   ` Alistair Francis
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 03/10] hw/sd/sd.c: Convert sd_reset() function into Device reset method Peter Maydell
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 17+ messages in thread
From: Peter Maydell @ 2016-01-22 16:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Crosthwaite, Sai Pavan Boddu, patches, Markus Armbruster,
	Alistair Francis, Kevin O'Connor, qemu-arm,
	Edgar E. Iglesias, Paolo Bonzini

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 1a9935c..745e9d6 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -33,6 +33,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
 
@@ -77,6 +79,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;
@@ -472,34 +476,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)
@@ -1765,3 +1761,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] 17+ messages in thread

* [Qemu-devel] [PATCH v2 03/10] hw/sd/sd.c: Convert sd_reset() function into Device reset method
  2016-01-22 16:16 [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 01/10] hw/sd/sdhci.c: Remove x-drive property Peter Maydell
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 02/10] hw/sd/sd.c: QOMify Peter Maydell
@ 2016-01-22 16:16 ` Peter Maydell
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 04/10] hw/sd: Add QOM bus which SD cards plug in to Peter Maydell
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-01-22 16:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Crosthwaite, Sai Pavan Boddu, patches, Markus Armbruster,
	Alistair Francis, Kevin O'Connor, qemu-arm,
	Edgar E. Iglesias, Paolo Bonzini

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 745e9d6..b62f7f5 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -393,8 +393,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;
 
@@ -435,7 +436,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);
     }
 }
@@ -677,7 +678,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;
@@ -1783,8 +1784,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[] = {
@@ -1804,6 +1803,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] 17+ messages in thread

* [Qemu-devel] [PATCH v2 04/10] hw/sd: Add QOM bus which SD cards plug in to
  2016-01-22 16:16 [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (2 preceding siblings ...)
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 03/10] hw/sd/sd.c: Convert sd_reset() function into Device reset method Peter Maydell
@ 2016-01-22 16:16 ` Peter Maydell
  2016-02-08 23:24   ` Alistair Francis
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 05/10] hw/sd/sdhci.c: Update to use SDBus APIs Peter Maydell
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 17+ messages in thread
From: Peter Maydell @ 2016-01-22 16:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Crosthwaite, Sai Pavan Boddu, patches, Markus Armbruster,
	Alistair Francis, Kevin O'Connor, qemu-arm,
	Edgar E. Iglesias, Paolo Bonzini

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          |  46 +++++++++++++++--
 include/hw/sd/sd.h  |  62 ++++++++++++++++++++++
 4 files changed, 251 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 b62f7f5..1016aa5 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -430,14 +430,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);
+        }
     }
 }
 
@@ -1799,17 +1826,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] 17+ messages in thread

* [Qemu-devel] [PATCH v2 05/10] hw/sd/sdhci.c: Update to use SDBus APIs
  2016-01-22 16:16 [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (3 preceding siblings ...)
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 04/10] hw/sd: Add QOM bus which SD cards plug in to Peter Maydell
@ 2016-01-22 16:16 ` Peter Maydell
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 06/10] sdhci_sysbus: Create SD card device in users, not the device itself Peter Maydell
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-01-22 16:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Crosthwaite, Sai Pavan Boddu, patches, Markus Armbruster,
	Alistair Francis, Kevin O'Connor, qemu-arm,
	Edgar E. Iglesias, Paolo Bonzini

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 a692eee..ad3a91c 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] 17+ messages in thread

* [Qemu-devel] [PATCH v2 06/10] sdhci_sysbus: Create SD card device in users, not the device itself
  2016-01-22 16:16 [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (4 preceding siblings ...)
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 05/10] hw/sd/sdhci.c: Update to use SDBus APIs Peter Maydell
@ 2016-01-22 16:16 ` Peter Maydell
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 07/10] hw/sd/pxa2xx_mmci: convert to SysBusDevice object Peter Maydell
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-01-22 16:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Crosthwaite, Sai Pavan Boddu, patches, Markus Armbruster,
	Alistair Francis, Kevin O'Connor, qemu-arm,
	Edgar E. Iglesias, Paolo Bonzini

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 2cd69b5..d294581 100644
--- a/hw/arm/xlnx-ep108.c
+++ b/hw/arm/xlnx-ep108.c
@@ -64,6 +64,27 @@ static void xlnx_ep108_init(MachineState *machine)
         exit(1);
     }
 
+    /* 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 ad3a91c..f40f500 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] 17+ messages in thread

* [Qemu-devel] [PATCH v2 07/10] hw/sd/pxa2xx_mmci: convert to SysBusDevice object
  2016-01-22 16:16 [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (5 preceding siblings ...)
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 06/10] sdhci_sysbus: Create SD card device in users, not the device itself Peter Maydell
@ 2016-01-22 16:16 ` Peter Maydell
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 08/10] hw/sd/pxa2xx_mmci: Update to use new SDBus APIs Peter Maydell
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-01-22 16:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Crosthwaite, Sai Pavan Boddu, patches, Markus Armbruster,
	Alistair Francis, Kevin O'Connor, qemu-arm,
	Edgar E. Iglesias, Paolo Bonzini

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 b217080..30d3aa2 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -11,16 +11,24 @@
  */
 
 #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;
@@ -48,7 +56,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 */
@@ -474,31 +482,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] 17+ messages in thread

* [Qemu-devel] [PATCH v2 08/10] hw/sd/pxa2xx_mmci: Update to use new SDBus APIs
  2016-01-22 16:16 [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (6 preceding siblings ...)
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 07/10] hw/sd/pxa2xx_mmci: convert to SysBusDevice object Peter Maydell
@ 2016-01-22 16:16 ` Peter Maydell
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 09/10] hw/sd/pxa2xx_mmci: Convert to VMStateDescription Peter Maydell
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-01-22 16:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Crosthwaite, Sai Pavan Boddu, patches, Markus Armbruster,
	Alistair Francis, Kevin O'Connor, qemu-arm,
	Edgar E. Iglesias, Paolo Bonzini

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 30d3aa2..9690c69 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -16,10 +16,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;
 
@@ -27,9 +31,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;
@@ -129,7 +135,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 --;
@@ -139,7 +145,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;
         }
@@ -173,7 +179,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));
@@ -482,32 +488,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)
@@ -525,6 +558,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 = {
@@ -534,9 +578,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] 17+ messages in thread

* [Qemu-devel] [PATCH v2 09/10] hw/sd/pxa2xx_mmci: Convert to VMStateDescription
  2016-01-22 16:16 [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (7 preceding siblings ...)
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 08/10] hw/sd/pxa2xx_mmci: Update to use new SDBus APIs Peter Maydell
@ 2016-01-22 16:16 ` Peter Maydell
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 10/10] hw/sd/pxa2xx_mmci: Add reset function Peter Maydell
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-01-22 16:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Crosthwaite, Sai Pavan Boddu, patches, Markus Armbruster,
	Alistair Francis, Kevin O'Connor, qemu-arm,
	Edgar E. Iglesias, Paolo Bonzini

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 9690c69..5170d79 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -43,27 +43,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 */
@@ -405,84 +450,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,
@@ -556,13 +523,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);
@@ -576,6 +547,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] 17+ messages in thread

* [Qemu-devel] [PATCH v2 10/10] hw/sd/pxa2xx_mmci: Add reset function
  2016-01-22 16:16 [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (8 preceding siblings ...)
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 09/10] hw/sd/pxa2xx_mmci: Convert to VMStateDescription Peter Maydell
@ 2016-01-22 16:16 ` Peter Maydell
  2016-02-08 11:33 ` [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
  2016-02-09  2:08 ` Kevin O'Connor
  11 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-01-22 16:16 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Crosthwaite, Sai Pavan Boddu, patches, Markus Armbruster,
	Alistair Francis, Kevin O'Connor, qemu-arm,
	Edgar E. Iglesias, Paolo Bonzini

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 5170d79..fd47f40 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -510,6 +510,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);
@@ -532,6 +561,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] 17+ messages in thread

* Re: [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci)
  2016-01-22 16:16 [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (9 preceding siblings ...)
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 10/10] hw/sd/pxa2xx_mmci: Add reset function Peter Maydell
@ 2016-02-08 11:33 ` Peter Maydell
  2016-02-09  2:08 ` Kevin O'Connor
  11 siblings, 0 replies; 17+ messages in thread
From: Peter Maydell @ 2016-02-08 11:33 UTC (permalink / raw)
  To: QEMU Developers
  Cc: Patch Tracking, Sai Pavan Boddu, Peter Crosthwaite,
	Markus Armbruster, Alistair Francis, Kevin O'Connor,
	qemu-arm, Paolo Bonzini, Edgar E. Iglesias

Ping?

thanks
-- PMM

On 22 January 2016 at 16:16, 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.
>
> 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
>
> If you want 'change' and 'eject' to work in the monitor you need to
> base on top of Max's patches fixing that regression.
> Git branch with that setup available at:
> https://git.linaro.org/people/peter.maydell/qemu-arm.git pxa-mmci-qomify
>
> 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            | 149 +++++++++++++++++++-----
>  hw/sd/sdhci.c         |  82 ++++++++------
>  include/hw/sd/sd.h    |  65 +++++++++++
>  include/hw/sd/sdhci.h |   3 +-
>  10 files changed, 623 insertions(+), 176 deletions(-)
>  create mode 100644 hw/sd/core.c
>
> --
> 1.9.1
>
>

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

* Re: [Qemu-devel] [PATCH v2 02/10] hw/sd/sd.c: QOMify
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 02/10] hw/sd/sd.c: QOMify Peter Maydell
@ 2016-02-08 23:06   ` Alistair Francis
  0 siblings, 0 replies; 17+ messages in thread
From: Alistair Francis @ 2016-02-08 23:06 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-devel@nongnu.org Developers, Patch Tracking,
	Sai Pavan Boddu, Peter Crosthwaite, Markus Armbruster,
	Alistair Francis, Kevin O'Connor, qemu-arm, Paolo Bonzini,
	Edgar E. Iglesias

On Fri, Jan 22, 2016 at 8:16 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 1a9935c..745e9d6 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -33,6 +33,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
>
> @@ -77,6 +79,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;
> @@ -472,34 +476,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)
> @@ -1765,3 +1761,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] 17+ messages in thread

* Re: [Qemu-devel] [PATCH v2 04/10] hw/sd: Add QOM bus which SD cards plug in to
  2016-01-22 16:16 ` [Qemu-devel] [PATCH v2 04/10] hw/sd: Add QOM bus which SD cards plug in to Peter Maydell
@ 2016-02-08 23:24   ` Alistair Francis
  2016-02-08 23:37     ` Peter Maydell
  0 siblings, 1 reply; 17+ messages in thread
From: Alistair Francis @ 2016-02-08 23:24 UTC (permalink / raw)
  To: Peter Maydell
  Cc: qemu-devel@nongnu.org Developers, Patch Tracking,
	Sai Pavan Boddu, Peter Crosthwaite, Markus Armbruster,
	Alistair Francis, Kevin O'Connor, qemu-arm, Paolo Bonzini,
	Edgar E. Iglesias

On Fri, Jan 22, 2016 at 8:16 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>

This patch doesn't compile and neither does anything after this. I get
the following errors:

In file included from hw/sd/sd.c:35:0:
/work/alistai/master-qemu/include/hw/sd/sd.h:81:5: error: unknown type
name ‘DeviceClass’
     DeviceClass parent_class;
     ^
/work/alistai/master-qemu/include/hw/sd/sd.h:99:14: error: field
‘qbus’ has incomplete type
     BusState qbus;
              ^
/work/alistai/master-qemu/include/hw/sd/sd.h:104:14: error: field
‘parent_class’ has incomplete type
     BusClass parent_class;
              ^
/work/alistai/master-qemu/rules.mak:57: recipe for target 'hw/sd/sd.o' failed
make: *** [hw/sd/sd.o] Error 1
make: *** Waiting for unfinished jobs....

With this configuration:

./configure --target-list="arm-softmmu,aarch64-softmmu,aarch64-linux-user,microblazeel-softmmu,microblazeel-linux-user"
--enable-fdt --disable-werror --enable-sdl

Thanks,

Alistair

> ---
>  hw/sd/Makefile.objs |   2 +-
>  hw/sd/core.c        | 146 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/sd/sd.c          |  46 +++++++++++++++--
>  include/hw/sd/sd.h  |  62 ++++++++++++++++++++++
>  4 files changed, 251 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 b62f7f5..1016aa5 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -430,14 +430,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);
> +        }
>      }
>  }
>
> @@ -1799,17 +1826,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] 17+ messages in thread

* Re: [Qemu-devel] [PATCH v2 04/10] hw/sd: Add QOM bus which SD cards plug in to
  2016-02-08 23:24   ` Alistair Francis
@ 2016-02-08 23:37     ` Peter Maydell
  2016-02-08 23:45       ` Alistair Francis
  0 siblings, 1 reply; 17+ messages in thread
From: Peter Maydell @ 2016-02-08 23:37 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Patch Tracking, Sai Pavan Boddu, Peter Crosthwaite,
	Markus Armbruster, qemu-devel@nongnu.org Developers,
	Kevin O'Connor, qemu-arm, Paolo Bonzini, Edgar E. Iglesias

On 8 February 2016 at 23:24, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> This patch doesn't compile and neither does anything after this. I get
> the following errors:
>
> In file included from hw/sd/sd.c:35:0:
> /work/alistai/master-qemu/include/hw/sd/sd.h:81:5: error: unknown type
> name ‘DeviceClass’
>      DeviceClass parent_class;
>      ^
> /work/alistai/master-qemu/include/hw/sd/sd.h:99:14: error: field
> ‘qbus’ has incomplete type
>      BusState qbus;
>               ^
> /work/alistai/master-qemu/include/hw/sd/sd.h:104:14: error: field
> ‘parent_class’ has incomplete type
>      BusClass parent_class;
>               ^
> /work/alistai/master-qemu/rules.mak:57: recipe for target 'hw/sd/sd.o' failed
> make: *** [hw/sd/sd.o] Error 1
> make: *** Waiting for unfinished jobs....

Weird. Maybe something's changed in include/ since I sent the series
so we're no longer getting qdev.h implicitly pulled in by some other
header.

Does adding #include "hw/qdev.h" before the sd.h include in
sd.c fix this?

thanks
-- PMM

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

* Re: [Qemu-devel] [PATCH v2 04/10] hw/sd: Add QOM bus which SD cards plug in to
  2016-02-08 23:37     ` Peter Maydell
@ 2016-02-08 23:45       ` Alistair Francis
  0 siblings, 0 replies; 17+ messages in thread
From: Alistair Francis @ 2016-02-08 23:45 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Peter Crosthwaite, Sai Pavan Boddu, Patch Tracking,
	qemu-devel@nongnu.org Developers, Markus Armbruster,
	Kevin O'Connor, qemu-arm, Edgar E. Iglesias, Paolo Bonzini,
	Alistair Francis

On Mon, Feb 8, 2016 at 3:37 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 8 February 2016 at 23:24, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> This patch doesn't compile and neither does anything after this. I get
>> the following errors:
>>
>> In file included from hw/sd/sd.c:35:0:
>> /work/alistai/master-qemu/include/hw/sd/sd.h:81:5: error: unknown type
>> name ‘DeviceClass’
>>      DeviceClass parent_class;
>>      ^
>> /work/alistai/master-qemu/include/hw/sd/sd.h:99:14: error: field
>> ‘qbus’ has incomplete type
>>      BusState qbus;
>>               ^
>> /work/alistai/master-qemu/include/hw/sd/sd.h:104:14: error: field
>> ‘parent_class’ has incomplete type
>>      BusClass parent_class;
>>               ^
>> /work/alistai/master-qemu/rules.mak:57: recipe for target 'hw/sd/sd.o' failed
>> make: *** [hw/sd/sd.o] Error 1
>> make: *** Waiting for unfinished jobs....
>
> Weird. Maybe something's changed in include/ since I sent the series
> so we're no longer getting qdev.h implicitly pulled in by some other
> header.

I did rebase the patches ontop of the latest master, so that might be it.

>
> Does adding #include "hw/qdev.h" before the sd.h include in
> sd.c fix this?

Yeah, that fixes it.

Thanks,

Alistair

>
> thanks
> -- PMM
>

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

* Re: [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci)
  2016-01-22 16:16 [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
                   ` (10 preceding siblings ...)
  2016-02-08 11:33 ` [Qemu-devel] [PATCH v2 00/10] hw/sd: QOMify sd.c (and pxa2xx_mmci) Peter Maydell
@ 2016-02-09  2:08 ` Kevin O'Connor
  11 siblings, 0 replies; 17+ messages in thread
From: Kevin O'Connor @ 2016-02-09  2:08 UTC (permalink / raw)
  To: Peter Maydell
  Cc: Peter Crosthwaite, qemu-devel, patches, Markus Armbruster,
	Alistair Francis, Sai Pavan Boddu, qemu-arm, Edgar E. Iglesias,
	Paolo Bonzini

On Fri, Jan 22, 2016 at 04:16:32PM +0000, Peter Maydell wrote:
> This series attempts to QOMify sd.c (the actual SD card model),
> including a proper QOM bus between the controller and the card.
> 
> 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
> 

FWIW, the series looks fine to me.

Thanks,
-Kevin

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

end of thread, other threads:[~2016-02-09  2:08 UTC | newest]

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

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).