On 01/14/2018 12:34 PM, Peter Maydell wrote: > On 14 January 2018 at 02:45, Philippe Mathieu-Daudé <f4...@amsat.org> wrote: >> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> >> --- >> hw/i2c/core.c | 12 ++++++------ >> 1 file changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/hw/i2c/core.c b/hw/i2c/core.c >> index 59068f157e..c84dbfb884 100644 >> --- a/hw/i2c/core.c >> +++ b/hw/i2c/core.c >> @@ -8,6 +8,7 @@ >> */ >> >> #include "qemu/osdep.h" >> +#include "qapi/error.h" >> #include "hw/i2c/i2c.h" >> >> typedef struct I2CNode I2CNode; >> @@ -276,16 +277,15 @@ const VMStateDescription vmstate_i2c_slave = { >> } >> }; >> >> -static int i2c_slave_qdev_init(DeviceState *dev) >> +static void i2c_slave_realize(DeviceState *dev, Error **errp) >> { >> I2CSlave *s = I2C_SLAVE(dev); >> I2CSlaveClass *sc = I2C_SLAVE_GET_CLASS(s); >> >> - if (sc->init) { >> - return sc->init(s); >> + if (sc->init && sc->init(s)) { >> + error_setg(errp, "i2c slave initialization failed"); >> + return; >> } >> - >> - return 0; >> } >> >> DeviceState *i2c_create_slave(I2CBus *bus, const char *name, uint8_t addr) >> @@ -301,7 +301,7 @@ DeviceState *i2c_create_slave(I2CBus *bus, const char >> *name, uint8_t addr) >> static void i2c_slave_class_init(ObjectClass *klass, void *data) >> { >> DeviceClass *k = DEVICE_CLASS(klass); >> - k->init = i2c_slave_qdev_init; >> + k->realize = i2c_slave_realize; >> set_bit(DEVICE_CATEGORY_MISC, k->categories); >> k->bus_type = TYPE_I2C_BUS; >> k->props = i2c_props; > > This is changing the semantics of the I2CSlaveClass::init > method. Is that really OK? (If nothing else, it means > that we end up with a method named init which is called > at realize time, which is confusing, and which doesn't > have an API like realize which allows it to fill in > an Error**.)
I see your point and missed it. I'll respin this patch once I2CSlaveClass is correctly converted to realize(). Thanks, Phil.