Commit 260bc9d8aa "hw/sd/sd.c: QOMify" QOMified only the device itself, not its users. It kept sd_init() around for non-QOMified users.
More than four years later, three such users remain: omap1 (machines cheetah, sx1, sx1-v1) and omap2 (machines n800, n810) are not QOMified, and pl181 (machines integratorcp, realview-eb, realview-eb-mpcore, realview-pb-a8 realview-pbx-a9, versatileab, versatilepb, vexpress-a15, vexpress-a9) is not QOMified properly. The issue I presently have with this: an "sd-card" device should plug into an "sd-bus" (its DeviceClass member bus_type says so), but sd_init() leaves it unplugged. This is normally a bug (I just fixed some instances), and I'd like to assert proper pluggedness to prevent regressions. However, the qdev-but-not-quite thing returned by sd_init() would fail the assertion. Meh. Make sd_init() hide it from QOM/qdev. Visible in "info qom-tree", here's the change for cheetah: /machine (cheetah-machine) [...] /unattached (container) [...] /device[5] (serial-mm) /serial (serial) /serial[0] (qemu:memory-region) - /device[6] (sd-card) - /device[7] (omap-gpio) + /device[6] (omap-gpio) [rest of device[*] renumbered...] Cc: "Philippe Mathieu-Daudé" <phi...@redhat.com> Signed-off-by: Markus Armbruster <arm...@redhat.com> --- hw/sd/sd.c | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/hw/sd/sd.c b/hw/sd/sd.c index 71a9af09ab..d7d8b82dfd 100644 --- a/hw/sd/sd.c +++ b/hw/sd/sd.c @@ -83,6 +83,10 @@ enum SDCardStates { struct SDState { DeviceState parent_obj; + /* If true, created by sd_init() for a non-qdevified caller */ + /* TODO purge them with fire */ + bool me_no_qdev_me_kill_mammoth_with_rocks; + /* SD Memory Card Registers */ uint32_t ocr; uint8_t scr[8]; @@ -129,6 +133,8 @@ struct SDState { bool cmd_line; }; +static void sd_realize(DeviceState *dev, Error **errp); + static const char *sd_state_name(enum SDCardStates state) { static const char *state_name[] = { @@ -590,7 +596,7 @@ static void sd_cardchange(void *opaque, bool load, Error **errp) { SDState *sd = opaque; DeviceState *dev = DEVICE(sd); - SDBus *sdbus = SD_BUS(qdev_get_parent_bus(dev)); + SDBus *sdbus; bool inserted = sd_get_inserted(sd); bool readonly = sd_get_readonly(sd); @@ -601,19 +607,17 @@ static void sd_cardchange(void *opaque, bool load, Error **errp) trace_sdcard_ejected(); } - /* 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 { + if (sd->me_no_qdev_me_kill_mammoth_with_rocks) { qemu_set_irq(sd->inserted_cb, inserted); if (inserted) { qemu_set_irq(sd->readonly_cb, readonly); } + } else { + sdbus = SD_BUS(qdev_get_parent_bus(dev)); + sdbus_set_inserted(sdbus, inserted); + if (inserted) { + sdbus_set_readonly(sdbus, readonly); + } } } @@ -697,6 +701,7 @@ SDState *sd_init(BlockBackend *blk, bool is_spi) { Object *obj; DeviceState *dev; + SDState *sd; Error *err = NULL; obj = object_new(TYPE_SD_CARD); @@ -707,13 +712,24 @@ SDState *sd_init(BlockBackend *blk, bool is_spi) return NULL; } qdev_prop_set_bit(dev, "spi", is_spi); - object_property_set_bool(obj, true, "realized", &err); + + /* + * Realizing the device properly would put it into the QOM + * composition tree even though it is not plugged into an + * appropriate bus. That's a no-no. Hide the device from + * QOM/qdev, and call its qdev realize callback directly. + */ + object_ref(obj); + object_unparent(obj); + sd_realize(dev, &err); if (err) { error_report("sd_init failed: %s", error_get_pretty(err)); return NULL; } - return SD_CARD(dev); + sd = SD_CARD(dev); + sd->me_no_qdev_me_kill_mammoth_with_rocks = true; + return sd; } void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert) -- 2.21.1