Message ID | 20180114024524.28821-1-f4bug@amsat.org |
---|---|
State | New |
Headers | show |
Series | hw/i2c: QOM'ify i2c slave | expand |
This description is more appropriate: "hw/i2c: convert i2c slave init() to realize()" On 01/13/2018 11:45 PM, Philippe Mathieu-Daudé wrote: > Signed-off-by: Philippe Mathieu-Daudé <f4bug@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; >
On 14 January 2018 at 02:45, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > Signed-off-by: Philippe Mathieu-Daudé <f4bug@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**.) thanks -- PMM
On 01/14/2018 12:34 PM, Peter Maydell wrote: > On 14 January 2018 at 02:45, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@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.
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;
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/i2c/core.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)