Re: [Qemu-devel] [PATCH 02/11] smbus_eeprom: replace SMBusDeviceClass::init by DeviceClass::reset

[Philippe Mathieu-Daudé]

On 01/19/2018 03:15 PM, Eduardo Habkost wrote:
> On Tue, Jan 16, 2018 at 10:15:46AM -0300, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>> ---
>>  hw/i2c/smbus_eeprom.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
>> index b13ec0fe7a..7e81ae4fe5 100644
>> --- a/hw/i2c/smbus_eeprom.c
>> +++ b/hw/i2c/smbus_eeprom.c
>> @@ -97,12 +97,11 @@ static uint8_t eeprom_read_data(SMBusDevice *dev, 
>> uint8_t cmd, int n)
>>      return eeprom_receive_byte(dev);
>>  }
>>  
>> -static int smbus_eeprom_initfn(SMBusDevice *dev)
>> +static void smbus_eeprom_reset(DeviceState *dev)
>>  {
>>      SMBusEEPROMDevice *eeprom = (SMBusEEPROMDevice *)dev;
>>  
>>      eeprom->offset = 0;
>> -    return 0;
>>  }
>>  
>>  static Property smbus_eeprom_properties[] = {
>> @@ -115,7 +114,7 @@ static void smbus_eeprom_class_initfn(ObjectClass 
>> *klass, void *data)
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>>      SMBusDeviceClass *sc = SMBUS_DEVICE_CLASS(klass);
>>  
>> -    sc->init = smbus_eeprom_initfn;
>> +    dc->reset = smbus_eeprom_reset;
> 
> Trying to track down when each of these methods is called:
> 
> sc->init (SMBusDeviceClass::init)
>  -> called by i2c_slave_qdev_init() (DeviceClass:init)
>    -> called by device_realize() (DeviceClass::realize)
>      -> called by device_set_realized()
>        -> QOM setter for "realized" property
>          -> (multiple callers)
> 
> (eww, so many indirections)
> 
> dc->reset (DeviceClass::reset)
>   -> called by device_reset()
>     -> (multiple callers)
> 
> (much better!)
> 
> 
> It looks like this changes how the device behaves.  Is this
> fixing a device emulation bug?  If not, why should the device set
> offset=0 on DeviceClass::reset, and not DeviceClass::realize?

You right, if this is a device bugfix (which I'm not sure) this is
unrelated to this series, so I'll just drop it.

> 
> 
>>      sc->quick_cmd = eeprom_quick_cmd;
>>      sc->send_byte = eeprom_send_byte;
>>      sc->receive_byte = eeprom_receive_byte;
>> -- 
>> 2.15.1
>>
>>
>