On Mon, Jan 16, 2023 at 01:23:01PM +0100, Philippe Mathieu-Daudé wrote:
> On 14/1/23 18:01, Peter Delevoryas wrote:
> > Signed-off-by: Peter Delevoryas <pe...@pjd.dev>
> > ---
> > hw/nvram/eeprom_at24c.c | 10 ++++++++++
> > include/hw/nvram/eeprom_at24c.h | 10 ++++++++++
> > 2 files changed, 20 insertions(+)
> > create mode 100644 include/hw/nvram/eeprom_at24c.h
>
> > +void at24c_eeprom_init(I2CBus *bus, uint8_t address, uint32_t rom_size)
> > +{
> > + I2CSlave *i2c_dev = i2c_slave_new("at24c-eeprom", address);
>
> Please use the type definition: TYPE_AT24C_EE.
>
> > + DeviceState *dev = DEVICE(i2c_dev);
> > +
> > + qdev_prop_set_uint32(dev, "rom-size", rom_size);
> > + i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
>
> Although the allocated object is somehow reachable from the i2c bus
> object, it would be simpler to deallocate allowing the parent to keep
> a reference to it. So consider this prototype instead:
>
> I2CSlave *at24c_eeprom_create(I2CBus *bus, uint8_t address,
> uint32_t rom_size);
>
Oh ok, yeah that sounds good. In this case, if I let the parent keep a
reference, maybe I shouldn't use i2c_slave_realize_and_unref, and just use
qdev_realize/etc (to avoid the unref?). I'll try just returning the pointer
from the function to start with though.
> > +}
