[Qemu-devel] [PATCH for-2.3 v2 0/2] sdhci: add "drive" property

[Qemu-devel] [PATCH for-2.3 v2 0/2] sdhci: add "drive" property

[Paolo Bonzini]

realize() methods aren't supposed to go on fishing expeditions for
backends.  For this reason, "-drive if=" options (apart from if=none)
are only handled at board creation time, and is not affected by -device.

The right way for devices to tie themselves to a -drive option is
"-device sdhci-pci,drive=xyz".  For 2.3 we can fix sdhci only, since
sdhci-pci is a new ABI.  The others can wait for later.

Paolo


Paolo Bonzini (2):
  sd: move blk_attach_dev_nofail to caller
  sdhci: add "drive" property

 hw/arm/xilinx_zynq.c      | 11 ++++++++++-
 hw/sd/milkymist-memcard.c |  3 +++
 hw/sd/omap_mmc.c          |  4 ++++
 hw/sd/pl181.c             |  7 ++++++-
 hw/sd/pxa2xx_mmci.c       |  4 ++++
 hw/sd/sd.c                |  1 -
 hw/sd/sdhci.c             |  6 ++----
 hw/sd/sdhci.h             |  1 +
 hw/sd/ssi-sd.c            |  7 ++++++-
 9 files changed, 36 insertions(+), 8 deletions(-)

-- 
2.3.3

[Qemu-devel] [PATCH for-2.3 v2 1/2] sd: move blk_attach_dev_nofail to caller

[Paolo Bonzini]

blk_attach_dev might not be necessary for devices that use a drive-type
property.  It is not appropriate to make the call unconditional in
sd_init.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/sd/milkymist-memcard.c | 3 +++
 hw/sd/omap_mmc.c          | 4 ++++
 hw/sd/pl181.c             | 7 ++++++-
 hw/sd/pxa2xx_mmci.c       | 4 ++++
 hw/sd/sd.c                | 1 -
 hw/sd/sdhci.c             | 7 ++++++-
 hw/sd/ssi-sd.c            | 7 ++++++-
 7 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 9661eaf..dc81b9b 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -257,6 +257,9 @@ static int milkymist_memcard_init(SysBusDevice *dev)
 
     dinfo = drive_get_next(IF_SD);
     blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
+    if (blk) {
+        blk_attach_dev_nofail(blk, s);
+    }
     s->card = sd_init(blk, false);
     if (s->card == NULL) {
         return -1;
diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
index d072dec..f169cb1 100644
--- a/hw/sd/omap_mmc.c
+++ b/hw/sd/omap_mmc.c
@@ -19,6 +19,7 @@
 #include "hw/hw.h"
 #include "hw/arm/omap.h"
 #include "hw/sd.h"
+#include "sysemu/block-backend.h"
 
 struct omap_mmc_s {
     qemu_irq irq;
@@ -621,6 +622,9 @@ struct omap_mmc_s *omap2_mmc_init(struct omap_target_agent_s *ta,
     omap_l4_attach(ta, 0, &s->iomem);
 
     /* Instantiate the storage */
+    if (blk) {
+        blk_attach_dev_nofail(blk, s);
+    }
     s->card = sd_init(blk, false);
     if (s->card == NULL) {
         exit(1);
diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index e704b6e..1b7a72f 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -484,6 +484,7 @@ static int pl181_init(SysBusDevice *sbd)
     DeviceState *dev = DEVICE(sbd);
     PL181State *s = PL181(dev);
     DriveInfo *dinfo;
+    BlockBackend *blk;
 
     memory_region_init_io(&s->iomem, OBJECT(s), &pl181_ops, s, "pl181", 0x1000);
     sysbus_init_mmio(sbd, &s->iomem);
@@ -491,7 +492,11 @@ static int pl181_init(SysBusDevice *sbd)
     sysbus_init_irq(sbd, &s->irq[1]);
     qdev_init_gpio_out(dev, s->cardstatus, 2);
     dinfo = drive_get_next(IF_SD);
-    s->card = sd_init(dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, false);
+    blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
+    if (blk) {
+        blk_attach_dev_nofail(blk, s);
+    }
+    s->card = sd_init(blk, false);
     if (s->card == NULL) {
         return -1;
     }
diff --git a/hw/sd/pxa2xx_mmci.c b/hw/sd/pxa2xx_mmci.c
index ac3ab39..0366414 100644
--- a/hw/sd/pxa2xx_mmci.c
+++ b/hw/sd/pxa2xx_mmci.c
@@ -14,6 +14,7 @@
 #include "hw/arm/pxa.h"
 #include "hw/sd.h"
 #include "hw/qdev.h"
+#include "sysemu/block-backend.h"
 
 struct PXA2xxMMCIState {
     MemoryRegion iomem;
@@ -538,6 +539,9 @@ PXA2xxMMCIState *pxa2xx_mmci_init(MemoryRegion *sysmem,
     memory_region_add_subregion(sysmem, base, &s->iomem);
 
     /* Instantiate the actual storage */
+    if (blk) {
+        blk_attach_dev_nofail(blk, s);
+    }
     s->card = sd_init(blk, false);
     if (s->card == NULL) {
         exit(1);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index f955265..794969b 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -494,7 +494,6 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
     sd->enable = true;
     sd_reset(sd, blk);
     if (sd->blk) {
-        blk_attach_dev_nofail(sd->blk, sd);
         blk_set_dev_ops(sd->blk, &sd_block_ops, sd);
     }
     vmstate_register(NULL, -1, &sd_vmstate, sd);
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 27b914a..ed60674 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1145,9 +1145,14 @@ static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
 static void sdhci_initfn(SDHCIState *s)
 {
     DriveInfo *di;
+    BlockBackend *blk;
 
     di = drive_get_next(IF_SD);
-    s->card = sd_init(di ? blk_by_legacy_dinfo(di) : NULL, false);
+    blk = di ? blk_by_legacy_dinfo(di) : NULL;
+    if (blk) {
+        blk_attach_dev_nofail(blk, s);
+    }
+    s->card = sd_init(blk, false);
     if (s->card == NULL) {
         exit(1);
     }
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index a71fbca..488b615 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -253,10 +253,15 @@ static int ssi_sd_init(SSISlave *d)
     DeviceState *dev = DEVICE(d);
     ssi_sd_state *s = FROM_SSI_SLAVE(ssi_sd_state, d);
     DriveInfo *dinfo;
+    BlockBackend *blk;
 
     s->mode = SSI_SD_CMD;
     dinfo = drive_get_next(IF_SD);
-    s->sd = sd_init(dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, true);
+    blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
+    if (blk) {
+        blk_attach_dev_nofail(blk, s);
+    }
+    s->sd = sd_init(blk, true);
     if (s->sd == NULL) {
         return -1;
     }
-- 
2.3.3

[Qemu-devel] [PATCH for-2.3 v2 2/2] sdhci: add "drive" property

[Paolo Bonzini]

Add a drive property that can be used with sdhci-pci (instead of the odd
"-drive if=sd,... -device sdhci-pci" that has no equivalent in the rest
of QEMU).  Then adjust the zynq platform to set it based on the DriveInfo
that it gets.

Note that in this case the BlockBackend is attached to the SDHCI controller.
This is not a problem; the ->dev field is really unused and the dev_opaque
still points to the SDState struct.

Required for 2.3, as it changes how users access the sdhci-pci device.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/arm/xilinx_zynq.c | 11 ++++++++++-
 hw/sd/sdhci.c        | 11 ++---------
 hw/sd/sdhci.h        |  1 +
 3 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 5c37521..3d25c1a 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -114,6 +114,7 @@ static void zynq_init(MachineState *machine)
     MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
     MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
     DeviceState *dev;
+    DriveInfo *dinfo;
     SysBusDevice *busdev;
     qemu_irq pic[64];
     Error *err = NULL;
@@ -172,7 +173,7 @@ static void zynq_init(MachineState *machine)
     vmstate_register_ram_global(ocm_ram);
     memory_region_add_subregion(address_space_mem, 0xFFFC0000, ocm_ram);
 
-    DriveInfo *dinfo = drive_get(IF_PFLASH, 0, 0);
+    dinfo = drive_get(IF_PFLASH, 0, 0);
 
     /* AMD */
     pflash_cfi02_register(0xe2000000, NULL, "zynq.pflash", FLASH_SIZE,
@@ -217,11 +218,19 @@ static void zynq_init(MachineState *machine)
     gem_init(&nd_table[1], 0xE000C000, pic[77-IRQ_OFFSET]);
 
     dev = qdev_create(NULL, "generic-sdhci");
+    dinfo = drive_get_next(IF_SD);
+    if (dinfo) {
+        qdev_prop_set_drive_nofail(dev, "drive", blk_by_legacy_dinfo(dinfo));
+    }
     qdev_init_nofail(dev);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xE0100000);
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[56-IRQ_OFFSET]);
 
     dev = qdev_create(NULL, "generic-sdhci");
+    dinfo = drive_get_next(IF_SD);
+    if (dinfo) {
+        qdev_prop_set_drive_nofail(dev, "drive", blk_by_legacy_dinfo(dinfo));
+    }
     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]);
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index ed60674..078b0bd 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1144,15 +1144,7 @@ static inline unsigned int sdhci_get_fifolen(SDHCIState *s)
 
 static void sdhci_initfn(SDHCIState *s)
 {
-    DriveInfo *di;
-    BlockBackend *blk;
-
-    di = drive_get_next(IF_SD);
-    blk = di ? blk_by_legacy_dinfo(di) : NULL;
-    if (blk) {
-        blk_attach_dev_nofail(blk, s);
-    }
-    s->card = sd_init(blk, false);
+    s->card = sd_init(s->blk, false);
     if (s->card == NULL) {
         exit(1);
     }
@@ -1222,6 +1214,7 @@ static Property sdhci_properties[] = {
     DEFINE_PROP_UINT32("capareg", SDHCIState, capareg,
             SDHC_CAPAB_REG_DEFAULT),
     DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
+    DEFINE_PROP_DRIVE("drive", SDHCIState, blk),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/sd/sdhci.h b/hw/sd/sdhci.h
index 3352d23..69ccf58 100644
--- a/hw/sd/sdhci.h
+++ b/hw/sd/sdhci.h
@@ -237,6 +237,7 @@ typedef struct SDHCIState {
         PCIDevice pcidev;
         SysBusDevice busdev;
     };
+    BlockBackend *blk;
     SDState *card;
     MemoryRegion iomem;
 
-- 
2.3.3