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

[Eduardo Habkost <ehabkost@redhat.com>]

On Tue, Jan 16, 2018 at 10:15:46AM -0300, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  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?


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

-- 
Eduardo