[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 3/5] hw/nvram/eeprom_at24c: Add init_rom field and at24c_e
From: |
Joel Stanley |
Subject: |
Re: [PATCH v3 3/5] hw/nvram/eeprom_at24c: Add init_rom field and at24c_eeprom_init_rom helper |
Date: |
Wed, 18 Jan 2023 02:22:01 +0000 |
On Tue, 17 Jan 2023 at 23:24, Peter Delevoryas <peter@pjd.dev> wrote:
>
> Allows users to specify binary data to initialize an EEPROM, allowing users to
> emulate data programmed at manufacturing time.
I like it. Is there somewhere sensible to add a description to the
code base? Perhaps as a comment to your new function?
>
> - 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 | 2 ++
> 2 files changed, 34 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));
I like the MIN here; good usability feature. It's worth documenting too.
> + }
> +
> 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..5c9149331144 100644
> --- a/include/hw/nvram/eeprom_at24c.h
> +++ b/include/hw/nvram/eeprom_at24c.h
> @@ -19,5 +19,7 @@
> * @bus, and drop the reference to it (the device is realized).
> */
> 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
> --
> 2.39.0
>
- [PATCH v3 0/5] hw/nvram/eeprom_at24c: Cleanup + FRUID EEPROM init example, Peter Delevoryas, 2023/01/17
- [PATCH v3 1/5] hw/arm: Extract at24c_eeprom_init helper from Aspeed and Nuvoton boards, Peter Delevoryas, 2023/01/17
- [PATCH v3 2/5] hw/arm/aspeed: Replace aspeed_eeprom_init with at24c_eeprom_init, Peter Delevoryas, 2023/01/17
- [PATCH v3 4/5] hw/arm/aspeed: Add aspeed_eeprom.c, Peter Delevoryas, 2023/01/17
- [PATCH v3 3/5] hw/nvram/eeprom_at24c: Add init_rom field and at24c_eeprom_init_rom helper, Peter Delevoryas, 2023/01/17
- Re: [PATCH v3 3/5] hw/nvram/eeprom_at24c: Add init_rom field and at24c_eeprom_init_rom helper,
Joel Stanley <=
- [PATCH v3 5/5] hw/nvram/eeprom_at24c: Make reset behavior more like hardware, Peter Delevoryas, 2023/01/17