| Message ID | 20191022162137.27161-24-marcandre.lureau@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Clean-ups: qom-ify serial and remove QDEV_PROP_PTR | expand |
On Tue, 22 Oct 2019 at 17:24, Marc-André Lureau <marcandre.lureau@redhat.com> wrote: > > Instead, set the initial data field directly. > > (the initial data is an array of 256 bytes. As I don't know if it may > change over time, I keep the pointer to original buffer as is, but it > might be worth to consider to copy it instead) All the callers to smbus_eeprom_init_one() allocate the memory for the initial data, populate it, pass the pointer to smbus_eeprom_init_one() and do not save the pointer anyway. So we effectively "own" the data -- we could choose to copy the data and make the callers free the memory instead. > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> I'd still like to know what the right QOM way to pass 256 bytes of constant data to a device as a property is. thanks -- PMM
Hi On Tue, Oct 22, 2019 at 7:19 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Tue, 22 Oct 2019 at 17:24, Marc-André Lureau > <marcandre.lureau@redhat.com> wrote: > > > > Instead, set the initial data field directly. > > > > (the initial data is an array of 256 bytes. As I don't know if it may > > change over time, I keep the pointer to original buffer as is, but it > > might be worth to consider to copy it instead) > > All the callers to smbus_eeprom_init_one() allocate the > memory for the initial data, populate it, pass the pointer > to smbus_eeprom_init_one() and do not save the pointer > anyway. So we effectively "own" the data -- we could choose > to copy the data and make the callers free the memory instead. > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > I'd still like to know what the right QOM way to pass > 256 bytes of constant data to a device as a property is. > A property with a uint list visitor is the closest thing we have I guess. We can probably have a specialized QObject to hold a fixed array, but string form will probably remain a list, I guess. I can try that, but this is quite complicated to pass 256 bytes internally..
diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c index 54c86a0112..533c728b3b 100644 --- a/hw/i2c/smbus_eeprom.c +++ b/hw/i2c/smbus_eeprom.c @@ -44,7 +44,7 @@ typedef struct SMBusEEPROMDevice { SMBusDevice smbusdev; uint8_t data[SMBUS_EEPROM_SIZE]; - void *init_data; + uint8_t *init_data; uint8_t offset; bool accessed; } SMBusEEPROMDevice; @@ -129,14 +129,14 @@ static void smbus_eeprom_reset(DeviceState *dev) static void smbus_eeprom_realize(DeviceState *dev, Error **errp) { + SMBusEEPROMDevice *eeprom = SMBUS_EEPROM(dev); + smbus_eeprom_reset(dev); + if (eeprom->init_data == NULL) { + error_setg(errp, "init_data cannot be NULL"); + } } -static Property smbus_eeprom_properties[] = { - DEFINE_PROP_PTR("data", SMBusEEPROMDevice, init_data), - DEFINE_PROP_END_OF_LIST(), -}; - static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -146,9 +146,8 @@ static void smbus_eeprom_class_initfn(ObjectClass *klass, void *data) dc->reset = smbus_eeprom_reset; sc->receive_byte = eeprom_receive_byte; sc->write_data = eeprom_write_data; - dc->props = smbus_eeprom_properties; dc->vmsd = &vmstate_smbus_eeprom; - /* Reason: pointer property "data" */ + /* Reason: init_data */ dc->user_creatable = false; } @@ -172,7 +171,7 @@ void smbus_eeprom_init_one(I2CBus *smbus, uint8_t address, uint8_t *eeprom_buf) dev = qdev_create((BusState *) smbus, TYPE_SMBUS_EEPROM); qdev_prop_set_uint8(dev, "address", address); - qdev_prop_set_ptr(dev, "data", eeprom_buf); + SMBUS_EEPROM(dev)->init_data = eeprom_buf; qdev_init_nofail(dev); }
Instead, set the initial data field directly. (the initial data is an array of 256 bytes. As I don't know if it may change over time, I keep the pointer to original buffer as is, but it might be worth to consider to copy it instead) Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- hw/i2c/smbus_eeprom.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-)