| Message ID | 20180116131555.14242-3-f4bug@amsat.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | qdev: remove DeviceClass::init/exit() | expand |
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 > >
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é <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? 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 >> >> >
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; sc->quick_cmd = eeprom_quick_cmd; sc->send_byte = eeprom_send_byte; sc->receive_byte = eeprom_receive_byte;
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- hw/i2c/smbus_eeprom.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)