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
- If -drive property is provided, it overrides init_rom data
Signed-off-by: Peter Delevoryas <peter@pjd.dev>
Reviewed-by: Joel Stanley <joel@jms.id.au>
---
hw/nvram/eeprom_at24c.c | 37 ++++++++++++++++++++++++++++-----
include/hw/nvram/eeprom_at24c.h | 16 ++++++++++++++
2 files changed, 48 insertions(+), 5 deletions(-)
diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c
index 98857e3626b9..f8d751fa278d 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,19 +134,38 @@ 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. */
+ 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)
{
EEPROMState *ee = AT24C_EE(dev);
+ 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;
+ }
+
if (ee->blk) {
int64_t len = blk_getlength(ee->blk);
@@ -163,6 +185,7 @@ static void at24c_eeprom_realize(DeviceState *dev, Error **errp)
}
ee->mem = g_malloc0(ee->rsize);
+
}
static
@@ -176,6 +199,10 @@ void at24c_eeprom_reset(DeviceState *state)
memset(ee->mem, 0, ee->rsize);
+ if (ee->init_rom) {
+ memcpy(ee->mem, ee->init_rom, MIN(ee->init_rom_size, ee->rsize));
+ }
+
if (ee->blk) {
int ret = blk_pread(ee->blk, 0, ee->rsize, ee->mem, 0);
diff --git a/include/hw/nvram/eeprom_at24c.h b/include/hw/nvram/eeprom_at24c.h
index 196db309d451..acb9857b2add 100644
--- a/include/hw/nvram/eeprom_at24c.h
+++ b/include/hw/nvram/eeprom_at24c.h
@@ -20,4 +20,20 @@
*/
I2CSlave *at24c_eeprom_init(I2CBus *bus, uint8_t address, uint32_t rom_size);
+
+/*
+ * Create and realize an AT24C EEPROM device on the heap with initial data.
+ * @bus: I2C bus to put it on
+ * @address: I2C address of the EEPROM slave when put on a bus
+ * @rom_size: size of the EEPROM
+ * @init_rom: Array of bytes to initialize EEPROM memory with
+ * @init_rom_size: Size of @init_rom, must be less than or equal to @rom_size
+ *
+ * Create the device state structure, initialize it, put it on the specified
+ * @bus, and drop the reference to it (the device is realized). Copies the data
+ * from @init_rom to the beginning of the EEPROM memory buffer.
+ */
+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
--
2.39.0On Tue, Jan 17, 2023 at 06:42:12PM -0800, 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
> - If -drive property is provided, it overrides init_rom data
>
> Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> ---
> hw/nvram/eeprom_at24c.c | 37 ++++++++++++++++++++++++++++-----
> include/hw/nvram/eeprom_at24c.h | 16 ++++++++++++++
> 2 files changed, 48 insertions(+), 5 deletions(-)
>
> diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c
> index 98857e3626b9..f8d751fa278d 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,19 +134,38 @@ 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);
Why did you switch from using i2c_slave_new()? Using it is more
documentation and future-proofing than convenience.
Other than that, looks good to me.
Reviewed-by: Corey Minyard <cminyard@mvista.com>
> + 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. */
> + 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)
> {
> EEPROMState *ee = AT24C_EE(dev);
>
> + 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;
> + }
> +
> if (ee->blk) {
> int64_t len = blk_getlength(ee->blk);
>
> @@ -163,6 +185,7 @@ static void at24c_eeprom_realize(DeviceState *dev, Error **errp)
> }
>
> ee->mem = g_malloc0(ee->rsize);
> +
> }
>
> static
> @@ -176,6 +199,10 @@ void at24c_eeprom_reset(DeviceState *state)
>
> memset(ee->mem, 0, ee->rsize);
>
> + if (ee->init_rom) {
> + memcpy(ee->mem, ee->init_rom, MIN(ee->init_rom_size, ee->rsize));
> + }
> +
> if (ee->blk) {
> int ret = blk_pread(ee->blk, 0, ee->rsize, ee->mem, 0);
>
> diff --git a/include/hw/nvram/eeprom_at24c.h b/include/hw/nvram/eeprom_at24c.h
> index 196db309d451..acb9857b2add 100644
> --- a/include/hw/nvram/eeprom_at24c.h
> +++ b/include/hw/nvram/eeprom_at24c.h
> @@ -20,4 +20,20 @@
> */
> I2CSlave *at24c_eeprom_init(I2CBus *bus, uint8_t address, uint32_t rom_size);
>
> +
> +/*
> + * Create and realize an AT24C EEPROM device on the heap with initial data.
> + * @bus: I2C bus to put it on
> + * @address: I2C address of the EEPROM slave when put on a bus
> + * @rom_size: size of the EEPROM
> + * @init_rom: Array of bytes to initialize EEPROM memory with
> + * @init_rom_size: Size of @init_rom, must be less than or equal to @rom_size
> + *
> + * Create the device state structure, initialize it, put it on the specified
> + * @bus, and drop the reference to it (the device is realized). Copies the data
> + * from @init_rom to the beginning of the EEPROM memory buffer.
> + */
> +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
> --
> 2.39.0
>
>
On Wed, Jan 25, 2023 at 03:36:23PM -0600, Corey Minyard wrote:
> On Tue, Jan 17, 2023 at 06:42:12PM -0800, 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
> > - If -drive property is provided, it overrides init_rom data
> >
> > Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> > Reviewed-by: Joel Stanley <joel@jms.id.au>
> > ---
> > hw/nvram/eeprom_at24c.c | 37 ++++++++++++++++++++++++++++-----
> > include/hw/nvram/eeprom_at24c.h | 16 ++++++++++++++
> > 2 files changed, 48 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c
> > index 98857e3626b9..f8d751fa278d 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,19 +134,38 @@ 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);
>
> 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.
- Peter
>
> Other than that, looks good to me.
>
> Reviewed-by: Corey Minyard <cminyard@mvista.com>
>
> > + 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. */
> > + 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)
> > {
> > EEPROMState *ee = AT24C_EE(dev);
> >
> > + 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;
> > + }
> > +
> > if (ee->blk) {
> > int64_t len = blk_getlength(ee->blk);
> >
> > @@ -163,6 +185,7 @@ static void at24c_eeprom_realize(DeviceState *dev, Error **errp)
> > }
> >
> > ee->mem = g_malloc0(ee->rsize);
> > +
> > }
> >
> > static
> > @@ -176,6 +199,10 @@ void at24c_eeprom_reset(DeviceState *state)
> >
> > memset(ee->mem, 0, ee->rsize);
> >
> > + if (ee->init_rom) {
> > + memcpy(ee->mem, ee->init_rom, MIN(ee->init_rom_size, ee->rsize));
> > + }
> > +
> > if (ee->blk) {
> > int ret = blk_pread(ee->blk, 0, ee->rsize, ee->mem, 0);
> >
> > diff --git a/include/hw/nvram/eeprom_at24c.h b/include/hw/nvram/eeprom_at24c.h
> > index 196db309d451..acb9857b2add 100644
> > --- a/include/hw/nvram/eeprom_at24c.h
> > +++ b/include/hw/nvram/eeprom_at24c.h
> > @@ -20,4 +20,20 @@
> > */
> > I2CSlave *at24c_eeprom_init(I2CBus *bus, uint8_t address, uint32_t rom_size);
> >
> > +
> > +/*
> > + * Create and realize an AT24C EEPROM device on the heap with initial data.
> > + * @bus: I2C bus to put it on
> > + * @address: I2C address of the EEPROM slave when put on a bus
> > + * @rom_size: size of the EEPROM
> > + * @init_rom: Array of bytes to initialize EEPROM memory with
> > + * @init_rom_size: Size of @init_rom, must be less than or equal to @rom_size
> > + *
> > + * Create the device state structure, initialize it, put it on the specified
> > + * @bus, and drop the reference to it (the device is realized). Copies the data
> > + * from @init_rom to the beginning of the EEPROM memory buffer.
> > + */
> > +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
> > --
> > 2.39.0
> >
> >
>>> 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 ?
Thanks,
C.
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.
On 1/18/23 03:42, 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
> - If -drive property is provided, it overrides init_rom data
>
> Signed-off-by: Peter Delevoryas <peter@pjd.dev>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Thanks,
C.
> ---
> hw/nvram/eeprom_at24c.c | 37 ++++++++++++++++++++++++++++-----
> include/hw/nvram/eeprom_at24c.h | 16 ++++++++++++++
> 2 files changed, 48 insertions(+), 5 deletions(-)
>
> diff --git a/hw/nvram/eeprom_at24c.c b/hw/nvram/eeprom_at24c.c
> index 98857e3626b9..f8d751fa278d 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,19 +134,38 @@ 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. */
> + 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)
> {
> EEPROMState *ee = AT24C_EE(dev);
>
> + 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;
> + }
> +
> if (ee->blk) {
> int64_t len = blk_getlength(ee->blk);
>
> @@ -163,6 +185,7 @@ static void at24c_eeprom_realize(DeviceState *dev, Error **errp)
> }
>
> ee->mem = g_malloc0(ee->rsize);
> +
> }
>
> static
> @@ -176,6 +199,10 @@ void at24c_eeprom_reset(DeviceState *state)
>
> memset(ee->mem, 0, ee->rsize);
>
> + if (ee->init_rom) {
> + memcpy(ee->mem, ee->init_rom, MIN(ee->init_rom_size, ee->rsize));
> + }
> +
> if (ee->blk) {
> int ret = blk_pread(ee->blk, 0, ee->rsize, ee->mem, 0);
>
> diff --git a/include/hw/nvram/eeprom_at24c.h b/include/hw/nvram/eeprom_at24c.h
> index 196db309d451..acb9857b2add 100644
> --- a/include/hw/nvram/eeprom_at24c.h
> +++ b/include/hw/nvram/eeprom_at24c.h
> @@ -20,4 +20,20 @@
> */
> I2CSlave *at24c_eeprom_init(I2CBus *bus, uint8_t address, uint32_t rom_size);
>
> +
> +/*
> + * Create and realize an AT24C EEPROM device on the heap with initial data.
> + * @bus: I2C bus to put it on
> + * @address: I2C address of the EEPROM slave when put on a bus
> + * @rom_size: size of the EEPROM
> + * @init_rom: Array of bytes to initialize EEPROM memory with
> + * @init_rom_size: Size of @init_rom, must be less than or equal to @rom_size
> + *
> + * Create the device state structure, initialize it, put it on the specified
> + * @bus, and drop the reference to it (the device is realized). Copies the data
> + * from @init_rom to the beginning of the EEPROM memory buffer.
> + */
> +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
© 2016 - 2026 Red Hat, Inc.