On Fri, Jan 27, 2023 at 08:42:40AM +0100, Cédric Le Goater wrote:
> > > > I2CSlave *at24c_eeprom_init(I2CBus *bus, uint8_t address, uint32_t
> > > > rom_size)
> > > > {
> > > > - I2CSlave *i2c_dev = i2c_slave_new(TYPE_AT24C_EE, address);
> > > > - DeviceState *dev = DEVICE(i2c_dev);
> > > > + return at24c_eeprom_init_rom(bus, address, rom_size, NULL, 0);
> > > > +}
> > > > +
> > > > +I2CSlave *at24c_eeprom_init_rom(I2CBus *bus, uint8_t address, uint32_t
> > > > rom_size,
> > > > + const uint8_t *init_rom, uint32_t
> > > > init_rom_size)
> > > > +{
> > > > + EEPROMState *s;
> > > > +
> > > > + s = AT24C_EE(qdev_new(TYPE_AT24C_EE));
> > > > +
> > > > + qdev_prop_set_uint8(DEVICE(s), "address", address);
> > >
> > > Why did you switch from using i2c_slave_new()? Using it is more
> > > documentation and future-proofing than convenience.
> >
> > Oh, yeah that's my bad. I was probably doing it so that all the
> > qdev_prop_set
> > calls to the object are in the same place, but I probably should have just
> > kept
> > i2c_slave_new() and initialized only the at24c-eeprom properties here,
> > instead
> > of initializing the I2CSlave ones too.
>
> Will you send a v5 ?
Oh! Yeah sure: I thought I had already missed the boat because I thought you
sent a pull request, but I see that it hasn't been pulled yet.
- Peter
>
> Thanks,
>
> C.
