On Mon, 18 May 2020, Markus Armbruster wrote:
sm501_init() and ati_vga_realize() create an "i2c-ddc" device, but
neglect to realize it.  Affects machines sam460ex, shix, r2d, and
fulong2e.

I wonder how this ever worked.  If the "device becomes real only on
realize" thing actually works, then we've always been missing the
device, yet nobody noticed.

No idea why it worked but guests can read EDID info fine with or without this patch, so

Tested-by: BALATON Zoltan <bala...@eik.bme.hu>

Maybe device is created and working after init as it has nothing special to do at realize (it doesn't even have a realize method) so all realize would do is to link it in qtree?

Regards,
BALATON Zoltan

Fix by realizing it right away.  Visible in "info qom-tree"; here's
the change for sam460ex:

    /machine (sam460ex-machine)
      [...]
      /unattached (container)
        [...]
   -    /device[14] (sii3112)
   +    /device[14] (i2c-ddc)
   +    /device[15] (sii3112)
        [rest of device[*] renumbered...]

Fixes: 4a1f253adb45ac6019971193d5077c4d5d55886a
Fixes: 4a1f253adb45ac6019971193d5077c4d5d55886a
Cc: BALATON Zoltan <bala...@eik.bme.hu>
Cc: qemu-...@nongnu.org
Cc: Magnus Damm <magnus.d...@gmail.com>
Cc: Philippe Mathieu-Daudé <f4...@amsat.org>
Cc: Aleksandar Markovic <aleksandar.qemu.de...@gmail.com>
Signed-off-by: Markus Armbruster <arm...@redhat.com>
---
hw/display/ati.c   | 1 +
hw/display/sm501.c | 1 +
2 files changed, 2 insertions(+)

diff --git a/hw/display/ati.c b/hw/display/ati.c
index 58ec8291d4..7c2177d7b3 100644
--- a/hw/display/ati.c
+++ b/hw/display/ati.c
@@ -929,6 +929,7 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp)
    bitbang_i2c_init(&s->bbi2c, i2cbus);
    I2CSlave *i2cddc = I2C_SLAVE(qdev_create(BUS(i2cbus), TYPE_I2CDDC));
    i2c_set_slave_address(i2cddc, 0x50);
+    qdev_init_nofail(DEVICE(i2cddc));

    /* mmio register space */
    memory_region_init_io(&s->mm, OBJECT(s), &ati_mm_ops, s,
diff --git a/hw/display/sm501.c b/hw/display/sm501.c
index acc692531a..132e75b641 100644
--- a/hw/display/sm501.c
+++ b/hw/display/sm501.c
@@ -1816,6 +1816,7 @@ static void sm501_init(SM501State *s, DeviceState *dev,
    /* ddc */
    I2CDDCState *ddc = I2CDDC(qdev_create(BUS(s->i2c_bus), TYPE_I2CDDC));
    i2c_set_slave_address(I2C_SLAVE(ddc), 0x50);
+    qdev_init_nofail(DEVICE(ddc));

    /* mmio */
    memory_region_init(&s->mmio_region, OBJECT(dev), "sm501.mmio", MMIO_SIZE);

Reply via email to