[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] hw/i2c: QOM'ify i2c slave
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH] hw/i2c: QOM'ify i2c slave |
Date: |
Sun, 14 Jan 2018 15:14:48 -0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 |
On 01/14/2018 12:34 PM, Peter Maydell wrote:
> On 14 January 2018 at 02:45, Philippe Mathieu-Daudé <address@hidden> wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>> ---
>> 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.