[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 3/5] hw/nvram/eeprom_at24c: Add init_rom field and at24c_e
From: |
Peter Delevoryas |
Subject: |
Re: [PATCH v2 3/5] hw/nvram/eeprom_at24c: Add init_rom field and at24c_eeprom_init_rom helper |
Date: |
Tue, 17 Jan 2023 09:57:24 -0800 |
On Tue, Jan 17, 2023 at 08:35:44AM +0100, Cédric Le Goater wrote:
> On 1/17/23 00:56, Peter Delevoryas wrote:
> > Allows users to specify binary data to initialize an EEPROM, allowing users
> > to
> > emulate data programmed at manufacturing time.
> >
> > - Added init_rom and init_rom_size attributes to TYPE_AT24C_EE
> > - Added at24c_eeprom_init_rom helper function to initialize attributes
> >
> > Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> > ---
> > hw/nvram/eeprom_at24c.c | 37 ++++++++++++++++++++++++++++-----
> > include/hw/nvram/eeprom_at24c.h | 2 ++
> > 2 files changed, 34 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c
> > index 98857e3626b9..bb9ee75864fe 100644
> > --- a/hw/nvram/eeprom_at24c.c
> > +++ b/hw/nvram/eeprom_at24c.c
> > @@ -50,6 +50,9 @@ struct EEPROMState {
> > uint8_t *mem;
> > BlockBackend *blk;
> > +
> > + const uint8_t *init_rom;
> > + uint32_t init_rom_size;
> > };
> > static
> > @@ -131,13 +134,26 @@ int at24c_eeprom_send(I2CSlave *s, uint8_t data)
> > 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);
> > + qdev_prop_set_uint32(DEVICE(s), "rom-size", rom_size);
> > - qdev_prop_set_uint32(dev, "rom-size", rom_size);
> > - i2c_slave_realize_and_unref(i2c_dev, bus, &error_abort);
> > + /* TODO: Model init_rom with QOM properties. */
>
> ok. This can be fixed with a property later when we have support.
>
> > + s->init_rom = init_rom;
> > + s->init_rom_size = init_rom_size;
> > - return i2c_dev;
> > + i2c_slave_realize_and_unref(I2C_SLAVE(s), bus, &error_abort);
> > +
> > + return I2C_SLAVE(s);
> > }
> > static void at24c_eeprom_realize(DeviceState *dev, Error **errp)
> > @@ -162,7 +178,14 @@ static void at24c_eeprom_realize(DeviceState *dev,
> > Error **errp)
> > }
> > }
> > + if (ee->init_rom_size > ee->rsize) {
> > + error_setg(errp, "%s: init rom is larger than rom: %u > %u",
> > + TYPE_AT24C_EE, ee->init_rom_size, ee->rsize);
> > + return;
> > + }
> > +
> > ee->mem = g_malloc0(ee->rsize);
> > +
> > }
> > static
> > @@ -185,6 +208,10 @@ void at24c_eeprom_reset(DeviceState *state)
> > }
> > DPRINTK("Reset read backing file\n");
> > }
> > +
> > + if (ee->init_rom) {
> > + memcpy(ee->mem, ee->init_rom, MIN(ee->init_rom_size, ee->rsize));
> > + }
>
> I don't think we can have both an init_rom and a drive. The realize
> handler should report an error in that case.
+1, I'll include that in v3.
>
> > }
> > static Property at24c_eeprom_props[] = {
> > diff --git a/include/hw/nvram/eeprom_at24c.h
> > b/include/hw/nvram/eeprom_at24c.h
> > index 79a36b53ca87..e490826ab1d0 100644
> > --- a/include/hw/nvram/eeprom_at24c.h
> > +++ b/include/hw/nvram/eeprom_at24c.h
> > @@ -6,5 +6,7 @@
> > #include "hw/i2c/i2c.h"
> > I2CSlave *at24c_eeprom_init(I2CBus *bus, uint8_t address, uint32_t
> > rom_size);
> > +I2CSlave *at24c_eeprom_init_rom(I2CBus *bus, uint8_t address, uint32_t
> > rom_size,
> > + const uint8_t *init_rom, uint32_t
> > init_rom_size);
> > #endif
>
[PATCH v2 5/5] hw/nvram/eeprom_at24c: Make reset behavior more like hardware, Peter Delevoryas, 2023/01/16